Better distinguish didFinishLoad and didStopLoading

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

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2011 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:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 275 matching lines @@
 
     if (!maybeCreateArchive()) {
         // If this is an empty document, it will not have actually been created yet. Commit dummy data so that
         // DocumentWriter::begin() gets called and creates the Document.
         if (!m_writer)
             commitData(0, 0);
     }
 
     endWriting(m_writer.get());
 
-    if (!m_mainDocumentError.isNull())
+    if (!m_mainDocumentError.isNull() || !m_frame)

[dcheng, 2015/10/08 05:34:35]

How can m_frame be null now?

[Nate Chapin, 2015/10/08 21:03:59]

It could be null when the DocumentLoader is synchronously detached inside
endWriting() before this change, too. It's m_mainDocumentError.isNull() that
changed.

Before:
* iframe completes, sends didFinishLoad, fires parent's load event.
* Parent's load event detached iframe, which calls DocumentLoader::stopLoading()
* DocumentLoader::finishedLoading() hasn't yet set m_state to MainResourceDone,
so it cancels the main resource load, which makes m_mainDocumentError non-null
as well as nulling m_frame and m_applicationCacheHost.
* Successful early exit.

After:
* iframe completes, sends didFinishLoad, ***sets DocumentLoader::m_state to
SentDidFinishLoad***, fires parent's load event.
* Parent's load event detached iframe, which calls DocumentLoader::stopLoading()
* DocumentLoader::isLoading() returns false because m_state is not in the right
range, main resource is not cancelled, m_mainDocumentError remains null.
* m_applicationCacheHost and m_frame were still both nulled, so we crash when we
unconditionally derefernce m_applicationCacheHost.

Moving m_applicationCacheHost->finishedLoadingMainResource() before
endWriting(), so that it is guaranteed non-null. The rest of the function is
benign when detached.

         return;
-    m_state = MainResourceDone;
-
-    // If the document specified an application cache manifest, it violates the author's intent if we store it in the memory cache
-    // and deny the appcache the chance to intercept it in the future, so remove from the memory cache.
-    if (m_frame) {
-        if (m_mainResource && m_frame->document()->hasAppCacheManifest())

[Nate Chapin, 2015/10/06 18:35:36]

This code should have been removed when we blinked, it's irrelevant because we
never store main resources in MemoryCache.

-            memoryCache()->remove(m_mainResource.get());
-    }
+    if (m_state < MainResourceDone)
+        m_state = MainResourceDone;
     m_applicationCacheHost->finishedLoadingMainResource();
     clearMainResourceHandle();
 }
 
 bool DocumentLoader::isRedirectAfterPost(const ResourceRequest& newRequest, const ResourceResponse& redirectResponse)
 {
     int status = redirectResponse.httpStatusCode();
     if (((status >= 301 && status <= 303) || status == 307)
         && m_originalRequest.httpMethod() == "POST")
         return true;
@@ skipping 559 matching lines @@
 {
     m_writer = createWriterFor(ownerDocument, init, mimeType(), m_writer ? m_writer->encoding() : emptyAtom, true, ForceSynchronousParsing);
     if (!source.isNull())
         m_writer->appendReplacingData(source);
     endWriting(m_writer.get());
 }
 
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader);
 
 } // namespace blink