bug 274173 

It can become null when the imports controller is disposed, so you're talking about a different shutdown protocol. What are you trying to achieve?

Sorry for lack of the details in the opening comment. I see lots of code that is calling Document::importsController()->master() on a Document that the class holds, and then doing something on the result, where its non-nullness (which is equivalent to not-yet-shutdowned-ness) is assumed. I thought we could simplify them.

E.g. FrameFetchContext::frameOfImportsController() (which I refactored recently) retrieves the associated LocalFrame from the master Document of the associated HTMLImportsController. Here, it's assumed that the returned Document is not yet nullptr, i.e. it's not yet disposed(). You see DCHECKs to guarantee that.

Document::shutdown() detaches the HTMLImportsController, yes. Code touching possibly detached HTMLImportsController may check ::master() to see if it's been detached or not. They're not in the scope of this issue.

What's confusing and I'm worried about is that we see lots of code that is taking care of this (retrieve the master Document when there's HTMLImportsController attached to the Document, otherwise, use the Document itself), and one without that logic. If the logic is really needed, we should encapsulate it into the Document and encourage use of it than implementing the logic in each caller. E.g. have Document::effectiveFrame() which returns m_frame or HTMLImportsController's LocalFrame if any.

But, the fact that master() is always the parent Document unless it's been shutdown makes me unsure if such effort is useful... it might make more sense to remove all these logic.

That's the motivation of this issue.

Hmm, OK. I see MediaValues::frameFrom, DocumentInit::frameForSecurityContext, etc. are the same thing, almost.

Sorry but after detailed discussion and review interactions with kochi@, I realized that I was misunderstanding.

I'd like to change this bug to be an umbrella for efforts clarifying how HTMLImportsController is used.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14595005f3e0ba925e5aca95d36e99789f657dee

commit 14595005f3e0ba925e5aca95d36e99789f657dee
Author: Takeshi Yoshino <tyoshino@chromium.org>
Date: Wed Aug 09 08:08:18 2017

Clean up around HTMLImportLoader construction

- Move the logic to ensure the Document has an HTMLImportsController to
  Document as EnsureImportsController().
- Remove the methods for debugging Show(), ShowTree() and ShowThis( )on
  the HTMLImport class which is no longer used even in the debug build.
- Remove HTMLImportChild::SetClient() and pass an HTMLImportChildClient
  directly via the constructor instead.
- Add const version of GetDocument() to the LinkResource class to
  reduce "owner_->" occurrences.

Bug:  673669 
Change-Id: I5b2584f34ad47d6983ebebe30746a2b9d419f464
Reviewed-on: https://chromium-review.googlesource.com/586218
Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492891}
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/dom/Document.h
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/LinkResource.cpp
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/LinkResource.h
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/HTMLImport.cpp
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/HTMLImport.h
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/HTMLImportChild.cpp
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/HTMLImportChild.h
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/HTMLImportsController.cpp
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/HTMLImportsController.h
[modify] https://crrev.com/14595005f3e0ba925e5aca95d36e99789f657dee/third_party/WebKit/Source/core/html/imports/LinkImport.cpp

I think it's clear enough for me. Thanks for your help.

Please feel free to use this for tracking your work if you want, kochi@. Otherwise, please close.

Thanks, as the original concern was resolved, closing.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e

commit 5b21d9f1d7105abb171ba16311d0f27dc7f7f09e
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Wed Aug 30 08:28:24 2017

Clean up usage of HTMLImportsController in Document

There are some common usages of Document.ImportsController() to
see if the document is an import or not, or if it is, to access
its master document.

It required some internal knowledge about how HTMLImportsController
is used for a master document and its imports, and made code hard
to understand.

Added some APIs in Document class so its users do not have to
access .ImportsController() directly.

Bug:  673669 ,  746150 
Change-Id: I4850c78988c224fb989045191f44f0f4d9ef0373
Reviewed-on: https://chromium-review.googlesource.com/571109
Commit-Queue: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Dominic Cooney <dominicc@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498396}
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/css/MediaValues.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/css/MediaValues.h
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/css/MediaValuesCached.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/css/MediaValuesDynamic.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/dom/Document.h
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/dom/StyleEngine.cpp
[modify] https://crrev.com/5b21d9f1d7105abb171ba16311d0f27dc7f7f09e/third_party/WebKit/Source/core/html/LinkResource.cpp