Revert of Make start/stop loading notifications per-frame (https://co…

…dereview.chromium.org/183793002/)

Reason for revert:
Possibly causing content_browsertests EndToEnd to fail.

See: http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%29/builds/2095/steps/content_browsertests/logs/stdio

Output:
[20917:20917:0327/122353:3268961829:INFO:CONSOLE(0)] "Uncaught TypeError: Cannot convert undefined or null to object", source: http://127.0.0.1:34379/files/web_ui_mojo.html (0)
[20917:20917:0327/122353:3269022437:INFO:CONSOLE(28)] "Uncaught TypeError: undefined is not a function", source: http://127.0.0.1:34379/files/web_ui_mojo.js (28)
BrowserTestBase signal handler received SIGTERM. Backtrace:
#0 0x000000a593be base::debug::StackTrace::StackTrace()
#1 0x000002bfc17b content::(anonymous namespace)::DumpStackTraceSignalHandler()
#2 0x7f9c3e4ab4a0 <unknown>
#3 0x7f9c3e55da43 __poll
#4 0x7f9c42aabff6 <unknown>
#5 0x7f9c42aac124 g_main_context_iteration
#6 0x000000aaaf5f base::MessagePumpGlib::RunWithDispatcher()
#7 0x000000a8e802 base::RunLoop::Run()
#8 0x00000058256e content::(anonymous namespace)::WebUIMojoTest_EndToEnd_Test::RunTestOnMainThread()

Original issue's description:
> Make start/stop loading notifications per-frame
> 
> Also, make ProgressTracker a frame-level concept rather than a page-level concept and merge it with FrameLoader's FrameProgressTracker helper. Send per-frame progress change notifications accordingly.
> 
> This depends on https://codereview.chromium.org/180113003/ landing in chromium.
> 
> BUG=347643
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170192

TBR=abarth@chromium.org,japhet@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=347643

Review URL: https://codereview.chromium.org/215533004

git-svn-id: svn://svn.chromium.org/blink/trunk@170218 bbb929c8-8fbe-4397-9dbb-9b2b20218538

Source/core/accessibility/AXRenderObject.cpp

@@ -1493,8 +1493,8 @@ double AXRenderObject::estimatedLoadingProgress() const
     if (isLoaded())
         return 1.0;
 
-    if (LocalFrame* frame = m_renderer->document().frame())
-        return frame->loader().progress().estimatedProgress();
+    if (Page* page = m_renderer->document().page())
+        return page->progress().estimatedProgress();
     return 0;
 }
 

Source/core/loader/EmptyClients.h

@@ -212,9 +212,9 @@ class EmptyFrameLoaderClient FINAL : public FrameLoaderClient {
     virtual void dispatchWillSendSubmitEvent(HTMLFormElement*) OVERRIDE;
     virtual void dispatchWillSubmitForm(HTMLFormElement*) OVERRIDE;
 
-    virtual void didStartLoading(LoadStartType) OVERRIDE { }
-    virtual void progressEstimateChanged(double) OVERRIDE { }
-    virtual void didStopLoading() OVERRIDE { }
+    virtual void postProgressStartedNotification(LoadStartType) OVERRIDE { }
+    virtual void postProgressEstimateChangedNotification() OVERRIDE { }
+    virtual void postProgressFinishedNotification() OVERRIDE { }
 
     virtual void loadURLExternally(const ResourceRequest&, NavigationPolicy, const String& = String()) OVERRIDE { }
 

Source/core/loader/FrameFetchContext.cpp

@@ -159,34 +159,39 @@ void FrameFetchContext::dispatchDidLoadResourceFromMemoryCache(const ResourceReq
 
 void FrameFetchContext::dispatchDidReceiveResponse(DocumentLoader* loader, unsigned long identifier, const ResourceResponse& r, ResourceLoader* resourceLoader)
 {
-    m_frame->loader().progress().incrementProgress(identifier, r);
+    if (Page* page = m_frame->page())
+        page->progress().incrementProgress(identifier, r);
     m_frame->loader().client()->dispatchDidReceiveResponse(loader, identifier, r);
     InspectorInstrumentation::didReceiveResourceResponse(m_frame, identifier, ensureLoader(loader), r, resourceLoader);
 }
 
 void FrameFetchContext::dispatchDidReceiveData(DocumentLoader*, unsigned long identifier, const char* data, int dataLength, int encodedDataLength)
 {
-    m_frame->loader().progress().incrementProgress(identifier, data, dataLength);
+    if (Page* page = m_frame->page())
+        page->progress().incrementProgress(identifier, data, dataLength);
     InspectorInstrumentation::didReceiveData(m_frame, identifier, data, dataLength, encodedDataLength);
 }
 
 void FrameFetchContext::dispatchDidDownloadData(DocumentLoader*, unsigned long identifier, int dataLength, int encodedDataLength)
 {
-    m_frame->loader().progress().incrementProgress(identifier, 0, dataLength);
+    if (Page* page = m_frame->page())
+        page->progress().incrementProgress(identifier, 0, dataLength);
     InspectorInstrumentation::didReceiveData(m_frame, identifier, 0, dataLength, encodedDataLength);
 }
 
 void FrameFetchContext::dispatchDidFinishLoading(DocumentLoader* loader, unsigned long identifier, double finishTime, int64_t encodedDataLength)
 {
-    m_frame->loader().progress().completeProgress(identifier);
+    if (Page* page = m_frame->page())
+        page->progress().completeProgress(identifier);
     m_frame->loader().client()->dispatchDidFinishLoading(loader, identifier);
 
     InspectorInstrumentation::didFinishLoading(m_frame, identifier, ensureLoader(loader), finishTime, encodedDataLength);
 }
 
 void FrameFetchContext::dispatchDidFail(DocumentLoader* loader, unsigned long identifier, const ResourceError& error)
 {
-    m_frame->loader().progress().completeProgress(identifier);
+    if (Page* page = m_frame->page())
+        page->progress().completeProgress(identifier);
     InspectorInstrumentation::didFailLoading(m_frame, identifier, error);
 }
 

Source/core/loader/FrameLoader.cpp

@@ -105,11 +105,48 @@ static bool needsHistoryItemRestore(FrameLoadType type)
     return type == FrameLoadTypeBackForward || type == FrameLoadTypeReload || type == FrameLoadTypeReloadFromOrigin;
 }
 
+class FrameLoader::FrameProgressTracker {
+public:
+    static PassOwnPtr<FrameProgressTracker> create(LocalFrame* frame) { return adoptPtr(new FrameProgressTracker(frame)); }
+    ~FrameProgressTracker()
+    {
+        ASSERT(!m_inProgress || m_frame->page());
+        if (m_inProgress)
+            m_frame->page()->progress().progressCompleted(m_frame);
+    }
+
+    void progressStarted()
+    {
+        ASSERT(m_frame->page());
+        if (!m_inProgress)
+            m_frame->page()->progress().progressStarted(m_frame);
+        m_inProgress = true;
+    }
+
+    void progressCompleted()
+    {
+        ASSERT(m_inProgress);
+        ASSERT(m_frame->page());
+        m_inProgress = false;
+        m_frame->page()->progress().progressCompleted(m_frame);
+    }
+
+private:
+    FrameProgressTracker(LocalFrame* frame)
+        : m_frame(frame)
+        , m_inProgress(false)
+    {
+    }
+
+    LocalFrame* m_frame;
+    bool m_inProgress;
+};
+
 FrameLoader::FrameLoader(LocalFrame* frame, FrameLoaderClient* client)
     : m_frame(frame)
     , m_client(client)
     , m_mixedContentChecker(frame)
-    , m_progressTracker(ProgressTracker::create(frame))
+    , m_progressTracker(FrameProgressTracker::create(m_frame))
     , m_state(FrameStateProvisional)
     , m_loadType(FrameLoadTypeStandard)
     , m_fetchContext(FrameFetchContext::create(frame))
@@ -536,14 +573,14 @@ void FrameLoader::updateForSameDocumentNavigation(const KURL& newURL, SameDocume
     // don't fire them for fragment redirection that happens in window.onload handler.
     // See https://bugs.webkit.org/show_bug.cgi?id=31838
     if (m_frame->document()->loadEventFinished())
-        m_client->didStartLoading(NavigationWithinSameDocument);
+        m_client->postProgressStartedNotification(NavigationWithinSameDocument);
 
     HistoryCommitType historyCommitType = updateBackForwardList == UpdateBackForwardList && m_currentItem ? StandardCommit : HistoryInertCommit;
     setHistoryItemStateForCommit(historyCommitType, sameDocumentNavigationSource == SameDocumentNavigationHistoryApi, data);
     m_client->dispatchDidNavigateWithinPage(m_currentItem.get(), historyCommitType);
     m_client->dispatchDidReceiveTitle(m_frame->document()->title());
     if (m_frame->document()->loadEventFinished())
-        m_client->didStopLoading();
+        m_client->postProgressFinishedNotification();
 }
 
 void FrameLoader::loadInSameDocument(const KURL& url, PassRefPtr<SerializedScriptValue> stateObject, UpdateBackForwardListPolicy updateBackForwardList, ClientRedirectPolicy clientRedirect)
@@ -1050,6 +1087,17 @@ void FrameLoader::checkLoadComplete(DocumentLoader* documentLoader)
     checkLoadComplete();
 }
 
+int FrameLoader::numPendingOrLoadingRequests(bool recurse) const
+{
+    if (!recurse)
+        return m_frame->document()->fetcher()->requestCount();
+
+    int count = 0;
+    for (LocalFrame* frame = m_frame; frame; frame = frame->tree().traverseNext(m_frame))
+        count += frame->document()->fetcher()->requestCount();
+    return count;
+}
+
 String FrameLoader::userAgent(const KURL& url) const
 {
     String userAgent = m_client->userAgent(url);

Source/core/loader/FrameLoader.h

@@ -60,7 +60,6 @@ class FrameLoaderClient;
 class IconController;
 class NavigationAction;
 class Page;
-class ProgressTracker;
 class ResourceError;
 class ResourceResponse;
 class SecurityOrigin;
@@ -83,7 +82,6 @@ class FrameLoader {
     LocalFrame* frame() const { return m_frame; }
 
     MixedContentChecker* mixedContentChecker() const { return &m_mixedContentChecker; }
-    ProgressTracker& progress() const { return *m_progressTracker; }
 
     // These functions start a load. All eventually call into loadWithNavigationAction() or loadInSameDocument().
     void load(const FrameLoadRequest&); // The entry point for non-reload, non-history loads.
@@ -113,6 +111,8 @@ class FrameLoader {
 
     bool isLoading() const;
 
+    int numPendingOrLoadingRequests(bool recurse) const;
+
     DocumentLoader* documentLoader() const { return m_documentLoader.get(); }
     DocumentLoader* policyDocumentLoader() const { return m_policyDocumentLoader.get(); }
     DocumentLoader* provisionalDocumentLoader() const { return m_provisionalDocumentLoader.get(); }
@@ -238,7 +238,8 @@ class FrameLoader {
     mutable FrameLoaderStateMachine m_stateMachine;
     mutable MixedContentChecker m_mixedContentChecker;
 
-    OwnPtr<ProgressTracker> m_progressTracker;
+    class FrameProgressTracker;
+    OwnPtr<FrameProgressTracker> m_progressTracker;
 
     FrameState m_state;
     FrameLoadType m_loadType;

Source/core/loader/FrameLoaderClient.h

@@ -122,9 +122,10 @@ namespace WebCore {
         virtual void dispatchWillSendSubmitEvent(HTMLFormElement*) = 0;
         virtual void dispatchWillSubmitForm(HTMLFormElement*) = 0;
 
-        virtual void didStartLoading(LoadStartType) = 0;
-        virtual void progressEstimateChanged(double progressEstimate) = 0;
-        virtual void didStopLoading() = 0;
+        // Maybe these should go into a ProgressTrackerClient some day
+        virtual void postProgressStartedNotification(LoadStartType) = 0;
+        virtual void postProgressEstimateChangedNotification() = 0;
+        virtual void postProgressFinishedNotification() = 0;
 
         virtual void loadURLExternally(const ResourceRequest&, NavigationPolicy, const String& suggestedName = String()) = 0;
 

Source/core/loader/ProgressTracker.cpp

@@ -26,7 +26,6 @@
 #include "config.h"
 #include "core/loader/ProgressTracker.h"
 
-#include "core/fetch/ResourceFetcher.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/inspector/InspectorInstrumentation.h"
@@ -62,29 +61,26 @@ struct ProgressItem {
     long long estimatedLength;
 };
 
-ProgressTracker::ProgressTracker(LocalFrame* frame)
-    : m_frame(frame)
-    , m_inProgress(false)
-    , m_totalPageAndResourceBytesToLoad(0)
+ProgressTracker::ProgressTracker()
+    : m_totalPageAndResourceBytesToLoad(0)
     , m_totalBytesReceived(0)
     , m_lastNotifiedProgressValue(0)
     , m_lastNotifiedProgressTime(0)
     , m_progressNotificationInterval(0.02)
     , m_progressNotificationTimeInterval(0.1)
     , m_finalProgressChangedSent(false)
     , m_progressValue(0)
+    , m_numProgressTrackedFrames(0)
 {
 }
 
 ProgressTracker::~ProgressTracker()
 {
-    if (m_inProgress)
-        progressCompleted();
 }
 
-PassOwnPtr<ProgressTracker> ProgressTracker::create(LocalFrame* frame)
+PassOwnPtr<ProgressTracker> ProgressTracker::create()
 {
-    return adoptPtr(new ProgressTracker(frame));
+    return adoptPtr(new ProgressTracker);
 }
 
 double ProgressTracker::estimatedProgress() const
@@ -102,35 +98,59 @@ void ProgressTracker::reset()
     m_lastNotifiedProgressValue = 0;
     m_lastNotifiedProgressTime = 0;
     m_finalProgressChangedSent = false;
+    m_numProgressTrackedFrames = 0;
+    m_originatingProgressFrame = nullptr;
 }
 
-void ProgressTracker::progressStarted()
+void ProgressTracker::progressStarted(LocalFrame* frame)
 {
-    if (!m_inProgress) {
+    WTF_LOG(Progress, "Progress started (%p) - frame %p(\"%s\"), value %f, tracked frames %d, originating frame %p", this, frame, frame->tree().uniqueName().utf8().data(), m_progressValue, m_numProgressTrackedFrames, m_originatingProgressFrame.get());
+
+    if (m_numProgressTrackedFrames == 0 || m_originatingProgressFrame == frame) {
         reset();
         m_progressValue = initialProgressValue;
-        m_frame->loader().client()->didStartLoading(NavigationToDifferentDocument);
+        m_originatingProgressFrame = frame;
+
+        m_originatingProgressFrame->loader().client()->postProgressStartedNotification(NavigationToDifferentDocument);
     }
-    m_inProgress = true;
-    InspectorInstrumentation::frameStartedLoading(m_frame);
+    m_numProgressTrackedFrames++;
+    InspectorInstrumentation::frameStartedLoading(frame);
+}
+
+void ProgressTracker::progressCompleted(LocalFrame* frame)
+{
+    WTF_LOG(Progress, "Progress completed (%p) - frame %p(\"%s\"), value %f, tracked frames %d, originating frame %p", this, frame, frame->tree().uniqueName().utf8().data(), m_progressValue, m_numProgressTrackedFrames, m_originatingProgressFrame.get());
+
+    if (m_numProgressTrackedFrames <= 0)
+        return;
+    m_numProgressTrackedFrames--;
+    if (!m_numProgressTrackedFrames || m_originatingProgressFrame == frame)
+        finalProgressComplete();
 }
 
-void ProgressTracker::progressCompleted()
+void ProgressTracker::finalProgressComplete()
 {
-    ASSERT(m_inProgress);
-    m_inProgress = false;
+    WTF_LOG(Progress, "Final progress complete (%p)", this);
+
+    RefPtr<LocalFrame> frame = m_originatingProgressFrame.release();
+
+    // Before resetting progress value be sure to send client a least one notification
+    // with final progress value.
     if (!m_finalProgressChangedSent) {
         m_progressValue = 1;
-        m_frame->loader().client()->progressEstimateChanged(m_progressValue);
+        frame->loader().client()->postProgressEstimateChangedNotification();
     }
+
     reset();
-    m_frame->loader().client()->didStopLoading();
-    InspectorInstrumentation::frameStoppedLoading(m_frame);
+    frame->loader().client()->postProgressFinishedNotification();
+    InspectorInstrumentation::frameStoppedLoading(frame.get());
 }
 
 void ProgressTracker::incrementProgress(unsigned long identifier, const ResourceResponse& response)
 {
-    if (!m_inProgress)
+    WTF_LOG(Progress, "Progress incremented (%p) - value %f, tracked frames %d, originating frame %p", this, m_progressValue, m_numProgressTrackedFrames, m_originatingProgressFrame.get());
+
+    if (m_numProgressTrackedFrames <= 0)
         return;
 
     long long estimatedLength = response.expectedContentLength();
@@ -154,6 +174,8 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
     if (!item)
         return;
 
+    RefPtr<LocalFrame> frame = m_originatingProgressFrame;
+
     unsigned bytesReceived = length;
     double increment, percentOfRemainingBytes;
     long long remainingBytes, estimatedBytesForPendingRequests;
@@ -164,7 +186,7 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
         item->estimatedLength = item->bytesReceived * 2;
     }
 
-    int numPendingOrLoadingRequests = m_frame->document()->fetcher()->requestCount();
+    int numPendingOrLoadingRequests = frame->loader().numPendingOrLoadingRequests(true);
     estimatedBytesForPendingRequests = progressItemDefaultEstimatedLength * numPendingOrLoadingRequests;
     remainingBytes = ((m_totalPageAndResourceBytesToLoad + estimatedBytesForPendingRequests) - m_totalBytesReceived);
     if (remainingBytes > 0)  // Prevent divide by 0.
@@ -173,7 +195,7 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
         percentOfRemainingBytes = 1.0;
 
     // For documents that use WebCore's layout system, treat first layout as the half-way point.
-    bool useClampedMaxProgress = !m_frame->view()->didFirstLayout();
+    bool useClampedMaxProgress = !frame->view()->didFirstLayout();
     double maxProgressValue = useClampedMaxProgress ? 0.5 : finalProgressValue;
     increment = (maxProgressValue - m_progressValue) * percentOfRemainingBytes;
     m_progressValue += increment;
@@ -185,13 +207,16 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
     double now = currentTime();
     double notifiedProgressTimeDelta = now - m_lastNotifiedProgressTime;
 
+    WTF_LOG(Progress, "Progress incremented (%p) - value %f, tracked frames %d", this, m_progressValue, m_numProgressTrackedFrames);
     double notificationProgressDelta = m_progressValue - m_lastNotifiedProgressValue;
-    if (notificationProgressDelta >= m_progressNotificationInterval || notifiedProgressTimeDelta >= m_progressNotificationTimeInterval) {
+    if ((notificationProgressDelta >= m_progressNotificationInterval ||
+         notifiedProgressTimeDelta >= m_progressNotificationTimeInterval) &&
+        m_numProgressTrackedFrames > 0) {
         if (!m_finalProgressChangedSent) {
             if (m_progressValue == 1)
                 m_finalProgressChangedSent = true;
 
-            m_frame->loader().client()->progressEstimateChanged(m_progressValue);
+            frame->loader().client()->postProgressEstimateChangedNotification();
 
             m_lastNotifiedProgressValue = m_progressValue;
             m_lastNotifiedProgressTime = now;