Remove FrameLoader's checkLoadComplete and checkLoadCompleteForThisFrame

Source/core/loader/FrameLoader.cpp

Index: Source/core/loader/FrameLoader.cpp
diff --git a/Source/core/loader/FrameLoader.cpp b/Source/core/loader/FrameLoader.cpp
index dc50a14425cea7863423ffa4dad060ba53a1865e..c941ae3d072a7e07e6e621c395bd56346169bbc1 100644
--- a/Source/core/loader/FrameLoader.cpp
+++ b/Source/core/loader/FrameLoader.cpp
@@ -247,10 +247,10 @@ bool FrameLoader::closeURL()
 void FrameLoader::didExplicitOpen()
 {
     // Calling document.open counts as committing the first real document load.
-    if (!m_stateMachine.committedFirstRealDocumentLoad()) {
+    if (!m_stateMachine.committedFirstRealDocumentLoad())
         m_stateMachine.advanceTo(FrameLoaderStateMachine::CommittedFirstRealLoad);
-        m_progressTracker->progressStarted();
-    }
+
+    m_progressTracker->progressStarted();

[Nate Chapin, 2015/01/12 18:39:30]

I should probably explicitly mention what this is doing. Traditionally, an
explicit open has returned the frame to a loading state based on state it
changes in Document. We are losing our former concept of "we started loading but
we didn't tell the embedder", which explicit open used. This means that a
document.open() or document.write() in a complete frame will send start
notifications to chromium. This may lead to the spinner in the browser UI
appearing more frequently.

[dcheng, 2015/01/13 00:20:52]

Does this mean a document.write() with no document.close() will result in an
infinite spinner?

[Nate Chapin, 2015/01/13 21:13:00]

I just tested it, and yes :(

Updated this patch to remove this hack and add a new one. Added
Frame::isLoadingInChild() and used it in allDescendantsAreComplete(), which
allows us to smuggle back in "document.write() blocks load events but doesn't
notify the embedder".

 
     // Prevent window.open(url) -- eg window.open("about:blank") -- from blowing away results
     // from a subsequent window.document.open / window.document.write call.
@@ -442,17 +442,10 @@ void FrameLoader::loadDone()
     checkCompleted();
 }
 
-bool FrameLoader::allChildrenAreComplete() const
+static bool allChildrenAreComplete(Frame* frame)
 {
-    for (Frame* child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling()) {
-        if (!child->isLocalFrame()) {
-            if (!child->checkLoadComplete()) {
-                return false;
-            }
-            continue;
-        }
-        LocalFrame* frame = toLocalFrame(child);
-        if (!frame->document()->isLoadCompleted() || frame->loader().m_provisionalDocumentLoader)
+    for (Frame* child = frame->tree().firstChild(); child; child = child->tree().traverseNext(frame)) {

[dcheng, 2015/01/09 22:52:25]

I assume you intentionally changed this from nextSibling() to traverseNext()? If
so, we should update this function to be called  allDescendentsAreComplete

[Nate Chapin, 2015/01/12 18:39:30]

Done.

+        if (child->isLoading())
             return false;
     }
     return true;
@@ -461,44 +454,57 @@ bool FrameLoader::allChildrenAreComplete() const
 bool FrameLoader::allAncestorsAreComplete() const
 {
     for (Frame* ancestor = m_frame; ancestor; ancestor = ancestor->tree().parent()) {
-        if (ancestor->isLocalFrame()) {
-            if (!toLocalFrame(ancestor)->document()->loadEventFinished())
-                return false;
-        } else {
-            if (!ancestor->checkLoadComplete()) {
-                return false;
-            }
-        }
-
+        if (ancestor->isLoading())
+            return false;
     }
     return true;
 }
 
-void FrameLoader::checkCompleted()
+static bool shouldComplete(Document* document)
 {
-    RefPtrWillBeRawPtr<LocalFrame> protect(m_frame.get());
+    if (document->parsing() || document->isInDOMContentLoaded())
+        return false;
+    if (!document->haveImportsLoaded())
+        return false;
+    if (document->fetcher()->requestCount())
+        return false;
+    if (document->isDelayingLoadEvent())
+        return false;
+    return allChildrenAreComplete(document->frame());
+}
 
-    if (m_frame->document()->isLoadCompleted() && m_stateMachine.committedFirstRealDocumentLoad())
-        return;
+static bool shouldSendCompleteNotifications(LocalFrame* frame)
+{
+    // Don't send stop notifications for inital empty documents, since they don't generate start notifications.
+    if (!frame->loader().stateMachine()->committedFirstRealDocumentLoad())
+        return false;
 
-    // Are we still parsing?
-    if (m_frame->document()->parsing() || m_frame->document()->isInDOMContentLoaded())
-        return;
+    // FIXME: We might have already sent stop notifications and be re-completing.
+    if (!frame->isLoading())
+        return false;
 
-    // Still waiting imports?
-    if (!m_frame->document()->haveImportsLoaded())
-        return;
+    // The readystatechanged or load event may have disconnected this frame.
+    if (!frame->client())
+        return false;
 
-    // Still waiting for images/scripts?
-    if (m_frame->document()->fetcher()->requestCount())
-        return;
+    // An event might have restarted a child frame.
+    if (!allChildrenAreComplete(frame))
+        return false;
 
-    // Still waiting for elements that don't go through a FrameLoader?
-    if (m_frame->document()->isDelayingLoadEvent())
-        return;
+    // An event might have restarted this frame by scheduling a new navigation.
+    if (frame->loader().provisionalDocumentLoader())
+        return false;
+
+    // We might have declined to run the load event due to an imminent content-initiated navigation.
+    if (!frame->document()->loadEventFinished())
+        return false;
+    return true;
+}
 
-    // Any frame that hasn't completed yet?
-    if (!allChildrenAreComplete())
+void FrameLoader::checkCompleted()
+{
+    RefPtrWillBeRawPtr<LocalFrame> protect(m_frame.get());
+    if (!shouldComplete(m_frame->document()))
         return;
 
     // OK, completed.
@@ -514,11 +520,20 @@ void FrameLoader::checkCompleted()
     if (m_frame->view())
         m_frame->view()->handleLoadCompleted();
 
+    if (shouldSendCompleteNotifications(m_frame)) {
+        m_loadType = FrameLoadTypeStandard;
+        m_progressTracker->progressCompleted();
+        m_frame->localDOMWindow()->finishedLoading();
+
+        // Report mobile vs. desktop page statistics. This will only report on Android.
+        if (m_frame->isMainFrame())
+            m_frame->document()->viewportDescription().reportMobilePageStats(m_frame);
+        client()->dispatchDidFinishLoad();
+    }
+
     Frame* parent = m_frame->tree().parent();
     if (parent && parent->isLocalFrame())
         toLocalFrame(parent)->loader().checkCompleted();
-    if (m_frame->page())
-        checkLoadComplete();
 }
 
 void FrameLoader::checkTimerFired(Timer<FrameLoader>*)
@@ -989,39 +1004,6 @@ FrameLoadType FrameLoader::loadType() const
     return m_loadType;
 }
 
-bool FrameLoader::checkLoadCompleteForThisFrame()
-{
-    ASSERT(client()->hasWebView());
-    RefPtrWillBeRawPtr<LocalFrame> protect(m_frame.get());
-
-    bool allChildrenAreDoneLoading = true;
-    for (RefPtrWillBeRawPtr<Frame> child = m_frame->tree().firstChild(); child; child = child->tree().nextSibling()) {
-        allChildrenAreDoneLoading &= child->checkLoadComplete();
-    }
-    if (!allChildrenAreDoneLoading)
-        return false;
-
-    if (!m_frame->isLoading())
-        return true;
-    if (m_provisionalDocumentLoader || !m_documentLoader)
-        return false;
-    if (!m_frame->document()->loadEventFinished())
-        return false;
-    if (!m_stateMachine.committedFirstRealDocumentLoad())
-        return true;
-
-    m_progressTracker->progressCompleted();
-    m_frame->localDOMWindow()->finishedLoading();
-
-    // Report mobile vs. desktop page statistics. This will only report on Android.
-    if (m_frame->isMainFrame())
-        m_frame->document()->viewportDescription().reportMobilePageStats(m_frame);
-
-    client()->dispatchDidFinishLoad();
-    m_loadType = FrameLoadTypeStandard;
-    return true;
-}
-
 void FrameLoader::restoreScrollPositionAndViewState()
 {
     FrameView* view = m_frame->view();
@@ -1069,13 +1051,6 @@ void FrameLoader::restoreScrollPositionAndViewState()
     }
 }
 
-// Called every time a resource is completely loaded or an error is received.
-void FrameLoader::checkLoadComplete()
-{
-    ASSERT(client()->hasWebView());
-    m_frame->page()->mainFrame()->checkLoadComplete();
-}
-
 String FrameLoader::userAgent(const KURL& url) const
 {
     String userAgent = client()->userAgent(url);
@@ -1132,10 +1107,7 @@ void FrameLoader::receivedMainResourceError(DocumentLoader* loader, const Resour
             m_progressTracker->progressCompleted();
         }
     }
-
     checkCompleted();
-    if (m_frame->page())
-        checkLoadComplete();
 }
 
 bool FrameLoader::shouldPerformFragmentNavigation(bool isFormSubmission, const String& httpMethod, FrameLoadType loadType, const KURL& url)