Let AssociatedURLLoader listen to destruction of the Document used for loading

third_party/WebKit/Source/web/AssociatedURLLoader.cpp

 /*
  * Copyright (C) 2010, 2011, 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 12 matching lines @@
  * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "web/AssociatedURLLoader.h"
 
+#include "core/dom/ActiveDOMObject.h"
 #include "core/fetch/CrossOriginAccessControl.h"
 #include "core/fetch/FetchUtils.h"
 #include "core/loader/DocumentThreadableLoader.h"
 #include "core/loader/DocumentThreadableLoaderClient.h"
 #include "platform/Timer.h"
 #include "platform/exported/WrappedResourceRequest.h"
 #include "platform/exported/WrappedResourceResponse.h"
 #include "platform/network/HTTPParsers.h"
 #include "platform/network/ResourceError.h"
 #include "public/platform/WebHTTPHeaderVisitor.h"
@@ skipping 237 matching lines @@
         m_errorTimer.startOneShot(0, BLINK_FROM_HERE);
 }
 
 void AssociatedURLLoader::ClientAdapter::notifyError(Timer<ClientAdapter>* timer)
 {
     ASSERT_UNUSED(timer, timer == &m_errorTimer);
 
     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();
 }
 
 #define STATIC_ASSERT_ENUM(a, b)                              \
     static_assert(static_cast<int>(a) == static_cast<int>(b), \
         "mismatching enum: " #a)
 
 STATIC_ASSERT_ENUM(WebURLLoaderOptions::CrossOriginRequestPolicyDeny, DenyCrossOriginRequests);
 STATIC_ASSERT_ENUM(WebURLLoaderOptions::CrossOriginRequestPolicyUseAccessControl, UseAccessControl);
 STATIC_ASSERT_ENUM(WebURLLoaderOptions::CrossOriginRequestPolicyAllow, AllowCrossOriginRequests);
 
 STATIC_ASSERT_ENUM(WebURLLoaderOptions::ConsiderPreflight, ConsiderPreflight);
 STATIC_ASSERT_ENUM(WebURLLoaderOptions::ForcePreflight, ForcePreflight);
 STATIC_ASSERT_ENUM(WebURLLoaderOptions::PreventPreflight, PreventPreflight);
 
 void AssociatedURLLoader::loadSynchronously(const WebURLRequest& request, WebURLResponse& response, WebURLError& error, WebData& data)
 {
     DCHECK(0); // Synchronous loading is not supported.
 }
 
 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);
     if (m_options.untrustedHTTP) {
         WebString method = newRequest.httpMethod();
         allowLoad = isValidHTTPToken(method) && FetchUtils::isUsefulMethod(method);
         if (allowLoad) {
             newRequest.setHTTPMethod(FetchUtils::normalizeMethod(method));
             HTTPRequestHeaderValidator validator;
             newRequest.visitHTTPHeaderFields(&validator);
             allowLoad = validator.isSafe();
         }
     }
 
-    m_clientAdapter = ClientAdapter::create(this, m_client, m_options);
+    m_clientAdapter = ClientAdapter::create(this, client, m_options);
 
     if (allowLoad) {
         ThreadableLoaderOptions options;
         options.preflightPolicy = static_cast<PreflightPolicy>(m_options.preflightPolicy);
         options.crossOriginRequestPolicy = static_cast<CrossOriginRequestPolicy>(m_options.crossOriginRequestPolicy);
 
         ResourceLoaderOptions resourceLoaderOptions;
         resourceLoaderOptions.allowCredentials = m_options.allowCredentials ? AllowStoredCredentials : DoNotAllowStoredCredentials;
         resourceLoaderOptions.dataBufferingPolicy = DoNotBufferData;
 
         const ResourceRequest& webcoreRequest = newRequest.toResourceRequest();
         if (webcoreRequest.requestContext() == WebURLRequest::RequestContextUnspecified) {
             // FIXME: We load URLs without setting a TargetType (and therefore a request context) in several
             // places in content/ (P2PPortAllocatorSession::AllocateLegacyRelaySession, for example). Remove
             // this once those places are patched up.
             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);
     }
 
     if (!m_loader) {
         // FIXME: return meaningful error codes.
         m_clientAdapter->setDelayedError(ResourceError());
     }
     m_clientAdapter->enableErrorNotifications();
 }
 
 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)
 {
     if (m_loader)
         m_loader->setDefersLoading(defersLoading);
 }
 
 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