Let AssociatedURLLoader listen to destruction of the Document used for loading

third_party/WebKit/Source/web/AssociatedURLLoader.cpp

Index: third_party/WebKit/Source/web/AssociatedURLLoader.cpp
diff --git a/third_party/WebKit/Source/web/AssociatedURLLoader.cpp b/third_party/WebKit/Source/web/AssociatedURLLoader.cpp
index 6bff6875e29b821319c6bf36287770f53535d84b..79637126f0a49ef58357c49e4a9d718109b9c186 100644
--- a/third_party/WebKit/Source/web/AssociatedURLLoader.cpp
+++ b/third_party/WebKit/Source/web/AssociatedURLLoader.cpp
@@ -30,6 +30,7 @@
 
 #include "web/AssociatedURLLoader.h"
 
+#include "core/dom/ActiveDOMObject.h"
 #include "core/fetch/CrossOriginAccessControl.h"
 #include "core/fetch/FetchUtils.h"
 #include "core/loader/DocumentThreadableLoader.h"
@@ -287,16 +288,45 @@ void AssociatedURLLoader::ClientAdapter::notifyError(Timer<ClientAdapter>* timer
     m_client->didFail(m_loader, m_error);
 }
 
+class AssociatedURLLoader::DocumentWatcher final : public GarbageCollectedFinalized<DocumentWatcher>, public ActiveDOMObject {

[yhirano, 2016/04/07 01:46:41]

My understanding is ContextLifecycleObserver is more preferable than
ActiveDOMObeject. https://bugs.chromium.org/p/chromium/issues/detail?id=589507
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/UwRGJgLBElo/...

[tyoshino (SeeGerritForStatus), 2016/04/07 05:54:06]

OK. Done

+    USING_GARBAGE_COLLECTED_MIXIN(DocumentWatcher);
+public:
+    DocumentWatcher(AssociatedURLLoader* parent, Document* document)
+        : ActiveDOMObject(document)
+        , m_parent(parent)
+    {
+        suspendIfNeeded();
+    }
+
+    void detach()
+    {
+        m_parent = nullptr;
+    }
+
+    void stop() override
+    {
+        if (m_parent)
+            m_parent->documentStopped();
+    }
+
+    DEFINE_INLINE_VIRTUAL_TRACE()
+    {
+        ActiveDOMObject::trace(visitor);
+    }
+
+    AssociatedURLLoader* m_parent;
+};
+
 AssociatedURLLoader::AssociatedURLLoader(RawPtr<WebLocalFrameImpl> frameImpl, const WebURLLoaderOptions& options)
-    : m_frameImpl(frameImpl)
+    : m_watcher(new DocumentWatcher(this, frameImpl->frame()->document()))
     , m_options(options)
-    , m_client(0)
 {
-    DCHECK(m_frameImpl);
 }
 
 AssociatedURLLoader::~AssociatedURLLoader()
 {
+    m_watcher->detach();
+
     cancel();
 }
 
@@ -320,10 +350,9 @@ void AssociatedURLLoader::loadSynchronously(const WebURLRequest& request, WebURL
 void AssociatedURLLoader::loadAsynchronously(const WebURLRequest& request, WebURLLoaderClient* client)
 {
     DCHECK(!m_loader);
-    DCHECK(!m_client);
+    DCHECK(!m_clientAdapter);
 
-    m_client = client;
-    DCHECK(m_client);
+    DCHECK(client);
 
     bool allowLoad = true;
     WebURLRequest newRequest(request);
@@ -338,7 +367,7 @@ void AssociatedURLLoader::loadAsynchronously(const WebURLRequest& request, WebUR
         }
     }
 
-    m_clientAdapter = ClientAdapter::create(this, m_client, m_options);
+    m_clientAdapter = ClientAdapter::create(this, client, m_options);
 
     if (allowLoad) {
         ThreadableLoaderOptions options;
@@ -357,9 +386,9 @@ void AssociatedURLLoader::loadAsynchronously(const WebURLRequest& request, WebUR
             newRequest.setRequestContext(WebURLRequest::RequestContextInternal);
         }
 
-        Document* webcoreDocument = m_frameImpl->frame()->document();
-        DCHECK(webcoreDocument);
-        m_loader = DocumentThreadableLoader::create(*webcoreDocument, m_clientAdapter.get(), options, resourceLoaderOptions);
+        Document* document = toDocument(m_watcher->getExecutionContext());
+        DCHECK(document);
+        m_loader = DocumentThreadableLoader::create(*document, m_clientAdapter.get(), options, resourceLoaderOptions);
         m_loader->start(webcoreRequest);
     }
 
@@ -372,10 +401,15 @@ void AssociatedURLLoader::loadAsynchronously(const WebURLRequest& request, WebUR
 
 void AssociatedURLLoader::cancel()
 {
-    if (m_clientAdapter)
-        m_clientAdapter->clearClient();
-    if (m_loader)
+    if (!m_clientAdapter)
+        return;
+
+    m_clientAdapter->clearClient();
+    if (m_loader) {
         m_loader->cancel();
+        m_loader.clear();
+    }
+    m_clientAdapter.clear();
 }
 
 void AssociatedURLLoader::setDefersLoading(bool defersLoading)
@@ -389,4 +423,12 @@ void AssociatedURLLoader::setLoadingTaskRunner(blink::WebTaskRunner*)
     // TODO(alexclarke): Maybe support this one day if it proves worthwhile.
 }
 
+void AssociatedURLLoader::documentStopped()

[yhirano, 2016/04/07 01:46:42]

Do we need to detach the watcher when a loading finishes?

[tyoshino (SeeGerritForStatus), 2016/04/07 05:54:06]

Good catch! Done

+{
+    if (m_clientAdapter)
+        m_clientAdapter->didFail(ResourceError());
+
+    cancel();
+}
+
 } // namespace blink