DocumentThreadableLoader: Add guards for sync notifyFinished() in setResource()

third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp

Index: third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
index 2e82f636155df855d69dd6a1789aed0c8d70f67b..0750ff0c81d7f292c1cf4fc671cf3ec52f3e3fe6 100644
--- a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
@@ -842,6 +842,8 @@ void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, Resou
             newRequest.setOriginRestriction(FetchRequest::NoOriginRestriction);
         ASSERT(!resource());
 
+        WeakPtr<DocumentThreadableLoader> self(m_weakFactory.createWeakPtr());
+
         if (request.requestContext() == WebURLRequest::RequestContextVideo || request.requestContext() == WebURLRequest::RequestContextAudio)
             setResource(RawResource::fetchMedia(newRequest, document().fetcher()));
         else if (request.requestContext() == WebURLRequest::RequestContextManifest)
@@ -849,9 +851,16 @@ void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, Resou
         else
             setResource(RawResource::fetch(newRequest, document().fetcher()));
 
+        // setResource() might call notifyFinished() synchronously, and thus
+        // clear() might be called and |this| may be dead here.
+        if (!self)
+            return;
+
         if (!resource()) {
             ThreadableLoaderClient* client = m_client;
             clear();
+            if (!client)
+                return;

[tyoshino (SeeGerritForStatus), 2016/04/19 06:27:44]

add comment to note that we need to call clear() for the case fetch.*() method
failed but in case clear() is called inside setResource() synchronously,
m_client is already cleared and therefore we need to return here.

[tyoshino (SeeGerritForStatus), 2016/04/19 06:29:00]

oh, basically the same as what you've put in the description.

[hiroshige, 2016/04/19 06:40:48]

Done.

[tyoshino (SeeGerritForStatus), 2016/04/19 08:31:32]

Thanks. LGTM

             client->didFail(ResourceError(errorDomainBlinkInternal, 0, requestURL.getString(), "Failed to start loading."));
             // |this| may be dead here.
             return;