Remove ResourceFetcher's access to a real FetchContext at frame detach time

Source/core/dom/Document.cpp

Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 6e1fa9a68d6010ed66d30b819750ce188235a70d..63b4370f8d4f1162b4f6e1580327a29d0ac32455 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -481,11 +481,13 @@ Document::Document(const DocumentInit& initializer, DocumentClassFlags documentC
         provideContextFeaturesToDocumentFrom(*this, *m_frame->page());
 
         m_fetcher = m_frame->loader().documentLoader()->fetcher();
-    }
-
-    if (!m_fetcher)
+        FrameFetchContext::provideDocumentToContext(m_fetcher->context(), this);
+    } else if (m_importsController) {
         m_fetcher = FrameFetchContext::createContextAndFetcher(nullptr);
-    static_cast<FrameFetchContext&>(m_fetcher->context()).setDocument(this);
+        FrameFetchContext::provideDocumentToContext(m_fetcher->context(), this);
+    } else {
+        m_fetcher = ResourceFetcher::create(nullptr);
+    }
 
     // We depend on the url getting immediately set in subframes, but we
     // also depend on the url NOT getting immediately set in opened windows.
@@ -530,6 +532,7 @@ Document::~Document()
 #if !ENABLE(OILPAN)
     ASSERT(m_ranges.isEmpty());
     ASSERT(!hasGuardRefCount());
+    ASSERT(!m_importsController);
     // With Oilpan, either the document outlives the visibility observers
     // or the visibility observers and the document die in the same GC round.
     // When they die in the same GC round, the list of visibility observers
@@ -564,9 +567,6 @@ Document::~Document()
     if (m_styleSheetList)
         m_styleSheetList->detachFromDocument();
 
-    if (m_importsController)
-        HTMLImportsController::removeFrom(*this);
-
     m_timeline->detachFromDocument();
 
     // We need to destroy CSSFontSelector before destroying m_fetcher.
@@ -575,13 +575,6 @@ Document::~Document()
     if (m_elemSheet)
         m_elemSheet->clearOwnerNode();
 
-    // It's possible for multiple Documents to end up referencing the same ResourceFetcher (e.g., SVGImages
-    // load the initial empty document and the SVGDocument with the same DocumentLoader).
-    FrameFetchContext& context = static_cast<FrameFetchContext&>(m_fetcher->context());
-    if (context.document() == this)
-        context.setDocument(nullptr);
-    m_fetcher.clear();
-
     // We must call clearRareData() here since a Document class inherits TreeScope
     // as well as Node. See a comment on TreeScope.h for the reason.
     if (hasRareData())
@@ -621,9 +614,6 @@ void Document::dispose()
 
     m_registrationContext.clear();
 
-    if (m_importsController)
-        HTMLImportsController::removeFrom(*this);
-
     // removeDetachedChildren() doesn't always unregister IDs,
     // so tear down scope information upfront to avoid having stale references in the map.
     destroyTreeScopeData();
@@ -832,6 +822,8 @@ void Document::setImportsController(HTMLImportsController* controller)
 {
     ASSERT(!m_importsController || !controller);
     m_importsController = controller;
+    if (!m_importsController)
+        m_fetcher->clearContext();
 }
 
 HTMLImportLoader* Document::importLoader() const
@@ -2136,6 +2128,19 @@ void Document::detach(const AttachContext& context)
 
     frameHost()->eventHandlerRegistry().documentDetached(*this);
 
+    // If this Document is associated with a live DocumentLoader, the
+    // DocumentLoader will take care of clearing the FetchContext. Deferring
+    // to the DocumentLoader when possible also prevents prematurely clearing
+    // the context in the case where multiple Documents end up associated with
+    // a single DocumentLoader (e.g., navigating to a javascript: url).
+    if (!loader())
+        m_fetcher->clearContext();
+    // If this document is the master for an HTMLImportsController, sever that
+    // relationship. This ensures that we don't leave import loads in flight,
+    // thinking they should have access to a valid frame when they don't.
+    if (m_importsController)
+        HTMLImportsController::removeFrom(*this);
+
     // This is required, as our LocalFrame might delete itself as soon as it detaches
     // us. However, this violates Node::detach() semantics, as it's never
     // possible to re-attach. Eventually Document::detach() should be renamed,