Better distinguish didFinishLoad and didStopLoading

Currently, they mean roughly the same thing. didFinishLoad
fires when a navigation successfully completes and no other
navigations are in progress. didStopLoading does the same
thing, but fires whether or not the navigation was successful.

This changes didFinishLoad to mean "a navigation successfully
completed", while didStopLoading still means "there are now
no navigations in progress in this frame". For each
didStartProvisionalLoad, there will be exactly one of:
didFailProvisionalLoad, didFailLoad, didFinishLoad. Prior to
this change, there would be at most one, but it could be that
a load would complete without a matching notification.

This also reverses the order of didFinishLoad and
didStopLoading. didStartLoading and didStopLoading will now
bracket zero to many individual navigation notifications,
which may be interleaved.

This requires 4 changes to consumers of these notifications:
1. http/tests/loading/progress-finished-callback.html
   asserted the old finish/stop ordering, and did nothing
   else. Remove it and its test harness logic.
2. web_navigation_api.cc assumed that didFinishLoad would not
   be sent if a new provisional navigation began before the
   finish notification. Change a DCHECK to an early exit.
3. password_autofill_agent.cc depends on stop firing before
   finish for determining whether all loading is done. Give
   it the ability to determine loading state for itself
   inside DidFinishLoad.
4. chrome/android/ incorrectly listens to the browser process
   equivalents of didStartProvisionaLoad and didFinishLoad to
   determine whether loading is in progress and start/stop
   the progress bar. Switch it to the equivalent of
   didStartLoading and didStopLoading.

BUG=539952
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:linux_perf_bisect;tryserver.chromium.perf:mac_10_10_perf_bisect;tryserver.chromium.perf:win_perf_bisect;tryserver.chromium.perf:android_nexus5_perf_bisect

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

Cr-Commit-Position: refs/heads/master@{#357517}

33 files changed, 179 insertions, 181 deletions

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
index f3b82e6..7af664f 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java
@@ -492,7 +492,7 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
             }
 
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 postDeferredStartupIfNeeded();
                 showUpdateInfoBarIfNecessary();
             }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
index 3d74cb9..eff42fc 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/OverlayPanelContent.java
@@ -132,8 +132,8 @@ public class OverlayPanelContent {
 
         mWebContentsDelegate = new WebContentsDelegateAndroid() {
             @Override
-            public void onLoadStarted() {
-                super.onLoadStarted();
+            public void onLoadStarted(boolean toDifferentDocument) {
+                super.onLoadStarted(toDifferentDocument);
                 mProgressObserver.onProgressBarStarted();
             }
 
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java
index 44751a6..6c31db9 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java
@@ -254,12 +254,12 @@ public class LayoutManagerChrome
 
         mTabSelectorTabObserver = new TabModelSelectorTabObserver(selector) {
             @Override
-            public void onLoadStarted(Tab tab) {
+            public void onLoadStarted(Tab tab, boolean toDifferentDocument) {
                 tabLoadStarted(tab.getId(), tab.isIncognito());
             }
 
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 tabLoadFinished(tab.getId(), tab.isIncognito());
             }
 
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/document/DocumentActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/document/DocumentActivity.java
index 5b4f551..a343040 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/document/DocumentActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/document/DocumentActivity.java
@@ -589,7 +589,7 @@ public class DocumentActivity extends ChromeActivity {
             }
 
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 assert mDocumentTab == tab;
 
                 updateTaskDescription();
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tab/EmptyTabObserver.java b/chrome/android/java/src/org/chromium/chrome/browser/tab/EmptyTabObserver.java
index 78b2811..3d06bf9 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/tab/EmptyTabObserver.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/tab/EmptyTabObserver.java
@@ -76,10 +76,10 @@ public class EmptyTabObserver implements TabObserver {
     public void onWebContentsInstantSupportDisabled() { }
 
     @Override
-    public void onLoadStarted(Tab tab) { }
+    public void onLoadStarted(Tab tab, boolean toDifferentDocument) { }
 
     @Override
-    public void onLoadStopped(Tab tab) { }
+    public void onLoadStopped(Tab tab, boolean toDifferentDocument) { }
 
     @Override
     public void onLoadProgressChanged(Tab tab, int progress) { }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java b/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
index fed11c8..27e6965 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/tab/Tab.java
@@ -1395,6 +1395,26 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener,
     }
 
     /**
+     * Called when a navigation begins and no navigation was in progress
+     * @param toDifferentDocument Whether this navigation will transition between
+     * documents (i.e., not a fragment navigation or JS History API call).
+     */
+    protected void onLoadStarted(boolean toDifferentDocument) {
+        if (toDifferentDocument) mIsLoading = true;
+        for (TabObserver observer : mObservers) observer.onLoadStarted(this, toDifferentDocument);
+    }
+
+    /**
+     * Called when a navigation completes and no other navigation is in progress.
+     */
+    protected void onLoadStopped() {
+        // mIsLoading should only be false if this is a same-document navigation.
+        boolean toDifferentDocument = mIsLoading;
+        mIsLoading = false;
+        for (TabObserver observer : mObservers) observer.onLoadStopped(this, toDifferentDocument);
+    }
+
+    /**
      * Called when a page has started loading.
      * @param validatedUrl URL being loaded.
      * @param showingErrorPage Whether an error page is being shown.
@@ -1403,7 +1423,6 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener,
         mIsFullscreenWaitingForLoad = !DomDistillerUrlUtils.isDistilledPage(validatedUrl);
 
         mIsShowingErrorPage = showingErrorPage;
-        mIsLoading = true;
 
         updateTitle();
         removeSadTabIfPresent();
@@ -1417,7 +1436,6 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener,
      * Called when a page has finished loading.
      */
     protected void didFinishPageLoad() {
-        mIsLoading = false;
         mIsBeingRestored = false;
         mIsTabStateDirty = true;
         updateTitle();
@@ -1450,7 +1468,6 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener,
      */
     protected void didFailPageLoad(int errorCode) {
         cancelEnableFullscreenLoadDelay();
-        mIsLoading = false;
         mIsBeingRestored = false;
         if (mTabUma != null) mTabUma.onLoadFailed(errorCode);
         for (TabObserver observer : mObservers) observer.onPageLoadFailed(this, errorCode);
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabObserver.java b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabObserver.java
index 6c1a32d..168fc12 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabObserver.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabObserver.java
@@ -173,14 +173,16 @@ public interface TabObserver {
      * staying in the same html document, {@link #onLoadStarted(Tab)} will be called, while
      * {@link #onPageLoadStarted(Tab, String)} will not.
      * @param tab The notifying {@link Tab}.
+     * @param toDifferentDocument Whether this navigation will transition between
+     * documents (i.e., not a fragment navigation or JS History API call).
      */
-    void onLoadStarted(Tab tab);
+    void onLoadStarted(Tab tab, boolean toDifferentDocument);
 
     /**
      * Called when the contents loading stops.
      * @param tab The notifying {@link Tab}.
      */
-    void onLoadStopped(Tab tab);
+    void onLoadStopped(Tab tab, boolean toDifferentDocument);
 
     /**
      * Called when the load progress of a {@link Tab} changes.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java
index 7b2b102..23946c5 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabWebContentsDelegateAndroid.java
@@ -185,19 +185,13 @@ public class TabWebContentsDelegateAndroid extends WebContentsDelegateAndroid {
     }
 
     @Override
-    public void onLoadStarted() {
-        RewindableIterator<TabObserver> observers = mTab.getTabObservers();
-        while (observers.hasNext()) {
-            observers.next().onLoadStarted(mTab);
-        }
+    public void onLoadStarted(boolean toDifferentDocument) {
+        mTab.onLoadStarted(toDifferentDocument);
     }
 
     @Override
     public void onLoadStopped() {
-        RewindableIterator<TabObserver> observers = mTab.getTabObservers();
-        while (observers.hasNext()) {
-            observers.next().onLoadStopped(mTab);
-        }
+        mTab.onLoadStopped();
     }
 
     @Override
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java
index 18483c0..d4baa88 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorImpl.java
@@ -176,7 +176,7 @@ public class TabModelSelectorImpl extends TabModelSelectorBase implements TabMod
             }
 
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 handleOnPageLoadStopped(tab);
             }
 
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java b/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
index 34b00f9..f419002 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
@@ -310,36 +310,12 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
 
             @Override
             public void onPageLoadStarted(Tab tab, String url) {
-                updateButtonStatus();
-                updateTabLoadingState(true);
-                mLoadProgressSimulator.cancel();
-
                 if (NativePageFactory.isNativePageUrl(url, tab.isIncognito())) {
                     finishLoadProgress(false);
-                } else {
-                    mToolbar.startLoadProgress();
-                    setLoadProgress(0.0f);
                 }
             }
 
             @Override
-            public void onPageLoadFinished(Tab tab) {
-                Tab currentTab = mToolbarModel.getTab();
-                updateTabLoadingState(true);
-
-                // If we made some progress, fast-forward to complete, otherwise just dismiss any
-                // MINIMUM_LOAD_PROGRESS that had been set.
-                if (currentTab.getProgress() > MINIMUM_LOAD_PROGRESS) setLoadProgress(1.0f);
-                finishLoadProgress(true);
-            }
-
-            @Override
-            public void onPageLoadFailed(Tab tab, int errorCode) {
-                updateTabLoadingState(true);
-                finishLoadProgress(false);
-            }
-
-            @Override
             public void onTitleUpdated(Tab tab) {
                 mLocationBar.setTitleToPageTitle();
             }
@@ -363,6 +339,30 @@ public class ToolbarManager implements ToolbarTabController, UrlFocusChangeListe
             }
 
             @Override
+            public void onLoadStarted(Tab tab, boolean toDifferentDocument) {
+                if (!toDifferentDocument) return;
+                updateButtonStatus();
+                updateTabLoadingState(true);
+                mLoadProgressSimulator.cancel();
+
+                mToolbar.startLoadProgress();
+                setLoadProgress(0.0f);
+            }
+
+            @Override
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
+                if (!toDifferentDocument) return;
+                updateTabLoadingState(true);
+
+                // If we made some progress, fast-forward to complete, otherwise just dismiss any
+                // MINIMUM_LOAD_PROGRESS that had been set.
+                if (tab.getProgress() > MINIMUM_LOAD_PROGRESS && tab.getProgress() < 1.0f) {
+                    setLoadProgress(1.0f);
+                }
+                finishLoadProgress(true);
+            }
+
+            @Override
             public void onLoadProgressChanged(Tab tab, int progress) {
                 // TODO(kkimlabs): Investigate using float progress all the way up to Blink.
                 setLoadProgress(progress / 100.0f);
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/GeolocationTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/GeolocationTest.java
index 6733cdc..32c7b70 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/GeolocationTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/GeolocationTest.java
@@ -92,7 +92,7 @@ public class GeolocationTest extends ChromeActivityTestCaseBase<ChromeActivity>
         final CallbackHelper loadCallback = new CallbackHelper();
         TabObserver observer = new EmptyTabObserver() {
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 // If the device has a cached non-mock location, we won't get back our
                 // lat/long, so checking that it has "#pass" is sufficient.
                 if (tab.getUrl().startsWith(url + "#pass|")) {
@@ -129,7 +129,7 @@ public class GeolocationTest extends ChromeActivityTestCaseBase<ChromeActivity>
         final CallbackHelper loadCallback0 = new CallbackHelper();
         TabObserver observer = new EmptyTabObserver() {
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 // If the device has a cached non-mock location, we won't get back our
                 // lat/long, so checking that it has "#pass" is sufficient.
                 if (tab.getUrl().startsWith(url + "#pass|0|")) {
@@ -153,7 +153,7 @@ public class GeolocationTest extends ChromeActivityTestCaseBase<ChromeActivity>
         final CallbackHelper loadCallback1 = new CallbackHelper();
         observer = new EmptyTabObserver() {
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 // If the device has a cached non-mock location, we won't get back our
                 // lat/long, so checking that it has "#pass" is sufficient.
                 if (tab.getUrl().startsWith(url + "#pass|1|")) {
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java
index 152f7c2..748cb1e 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java
@@ -179,7 +179,7 @@ public class HistoryUITest extends ChromeActivityTestCaseBase<ChromeActivity> {
         final CallbackHelper loadCallback = new CallbackHelper();
         TabObserver observer = new EmptyTabObserver() {
             @Override
-            public void onLoadStopped(Tab tab) {
+            public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
                 if (tab.getUrl().startsWith(HISTORY_URL)) {
                     loadCallback.notifyCalled();
                 }
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/BrandColorTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/BrandColorTest.java
index c0ac21a..04b8102 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/BrandColorTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/toolbar/BrandColorTest.java
@@ -147,7 +147,7 @@ public class BrandColorTest extends ChromeActivityTestCaseBase<ChromeActivity> {
             @Override
             public void run() {
                 getActivity().getActivityTab()
-                        .getTabWebContentsDelegateAndroid().onLoadStarted();
+                        .getTabWebContentsDelegateAndroid().onLoadStarted(true);
             }
         });
         checkForBrandColor(Color.parseColor(BRAND_COLOR_1));
diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
index 6597f0e..13bf2af 100644
--- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
+++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
@@ -386,12 +386,16 @@ void WebNavigationTabObserver::DidFinishLoad(
   navigation_state_.SetNavigationCompleted(render_frame_host);
   if (!navigation_state_.CanSendEvents(render_frame_host))
     return;
-  DCHECK(navigation_state_.GetUrl(render_frame_host) == validated_url ||
-         (navigation_state_.GetUrl(render_frame_host) ==
-              GURL(content::kAboutSrcDocURL) &&
-          validated_url == GURL(url::kAboutBlankURL)))
-      << "validated URL is " << validated_url << " but we expected "
-      << navigation_state_.GetUrl(render_frame_host);
+
+  // A new navigation might have started before the old one completed.
+  // Ignore the old navigation completion in that case.
+  // srcdoc iframes will report a url of about:blank, still let it through.
+  if (navigation_state_.GetUrl(render_frame_host) != validated_url &&
+      (navigation_state_.GetUrl(render_frame_host) !=
+           GURL(content::kAboutSrcDocURL) ||
+       validated_url != GURL(url::kAboutBlankURL))) {
+    return;
+  }
 
   // The load might already have finished by the time we finished parsing. For
   // compatibility reasons, we artifically delay the load completed signal until
diff --git a/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/TabLoadObserver.java b/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/TabLoadObserver.java
index 0e0ce2c..ca1b042 100644
--- a/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/TabLoadObserver.java
+++ b/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/TabLoadObserver.java
@@ -44,12 +44,12 @@ public class TabLoadObserver extends EmptyTabObserver implements Criteria {
     }
 
     @Override
-    public void onLoadStarted(Tab tab) {
+    public void onLoadStarted(Tab tab, boolean toDifferentDocument) {
         mTabLoadStarted = true;
     }
 
     @Override
-    public void onLoadStopped(Tab tab) {
+    public void onLoadStopped(Tab tab, boolean toDifferentDocument) {
         mTabLoadStopped = true;
     }
 
@@ -70,4 +70,4 @@ public class TabLoadObserver extends EmptyTabObserver implements Criteria {
         }
         return true;
     }
-}
\ No newline at end of file
+}
diff --git a/components/autofill/content/renderer/password_autofill_agent.cc b/components/autofill/content/renderer/password_autofill_agent.cc
index e049dfe..af47549 100644
--- a/components/autofill/content/renderer/password_autofill_agent.cc
+++ b/components/autofill/content/renderer/password_autofill_agent.cc
@@ -627,7 +627,6 @@ PasswordAutofillAgent::PasswordAutofillAgent(content::RenderFrame* render_frame)
       logging_state_active_(false),
       was_username_autofilled_(false),
       was_password_autofilled_(false),
-      did_stop_loading_(false),
       weak_ptr_factory_(this) {
   Send(new AutofillHostMsg_PasswordAutofillAgentConstructed(routing_id()));
 }
@@ -1050,9 +1049,16 @@ void PasswordAutofillAgent::SendPasswordForms(bool only_visible) {
   }
 
   if (only_visible) {
-    Send(new AutofillHostMsg_PasswordFormsRendered(routing_id(),
-                                                   password_forms,
-                                                   did_stop_loading_));
+    bool is_last_load = true;
+    for (blink::WebFrame* frame = render_frame()->GetWebFrame()->top(); frame;
+         frame = frame->traverseNext(false)) {
+      if (frame != render_frame()->GetWebFrame() && frame->isLoading()) {
+        is_last_load = false;
+        break;
+      }
+    }
+    Send(new AutofillHostMsg_PasswordFormsRendered(routing_id(), password_forms,
+                                                   is_last_load));
   } else {
     Send(new AutofillHostMsg_PasswordFormsParsed(routing_id(), password_forms));
   }
@@ -1098,14 +1104,6 @@ void PasswordAutofillAgent::DidCommitProvisionalLoad(
   }
 }
 
-void PasswordAutofillAgent::DidStartLoading() {
-  did_stop_loading_ = false;
-}
-
-void PasswordAutofillAgent::DidStopLoading() {
-  did_stop_loading_ = true;
-}
-
 void PasswordAutofillAgent::FrameDetached() {
   // If a sub frame has been destroyed while the user was entering information
   // into a password form, try to save the data. See https://crbug.com/450806
@@ -1524,14 +1522,6 @@ void PasswordAutofillAgent::LegacyPasswordAutofillAgent::OnDestruct() {
   // No op. Do not delete |this|.
 }
 
-void PasswordAutofillAgent::LegacyPasswordAutofillAgent::DidStartLoading() {
-  agent_->DidStartLoading();
-}
-
-void PasswordAutofillAgent::LegacyPasswordAutofillAgent::DidStopLoading() {
-  agent_->DidStopLoading();
-}
-
 void PasswordAutofillAgent::LegacyPasswordAutofillAgent::
     DidStartProvisionalLoad(blink::WebLocalFrame* navigated_frame) {
   agent_->LegacyDidStartProvisionalLoad(navigated_frame);
diff --git a/components/autofill/content/renderer/password_autofill_agent.h b/components/autofill/content/renderer/password_autofill_agent.h
index af38bf5..69f2bb5 100644
--- a/components/autofill/content/renderer/password_autofill_agent.h
+++ b/components/autofill/content/renderer/password_autofill_agent.h
@@ -151,8 +151,6 @@ class PasswordAutofillAgent : public content::RenderFrameObserver {
 
     // RenderViewObserver:
     void OnDestruct() override;
-    void DidStartLoading() override;
-    void DidStopLoading() override;
     void DidStartProvisionalLoad(blink::WebLocalFrame* frame) override;
 
    private:
@@ -174,8 +172,6 @@ class PasswordAutofillAgent : public content::RenderFrameObserver {
   void WillSubmitForm(const blink::WebFormElement& form) override;
 
   // Legacy RenderViewObserver:
-  void DidStartLoading();
-  void DidStopLoading();
   void LegacyDidStartProvisionalLoad(blink::WebLocalFrame* frame);
 
   // RenderView IPC handlers:
@@ -269,9 +265,6 @@ class PasswordAutofillAgent : public content::RenderFrameObserver {
   // Records the username typed before suggestions preview.
   base::string16 username_query_prefix_;
 
-  // True indicates that all frames in a page have been rendered.
-  bool did_stop_loading_;
-
   // Contains server predictions for username, password and/or new password
   // fields for individual forms.
   FormsPredictionsMap form_predictions_;
diff --git a/components/test_runner/test_runner.cc b/components/test_runner/test_runner.cc
index 6106761..996cd35 100644
--- a/components/test_runner/test_runner.cc
+++ b/components/test_runner/test_runner.cc
@@ -246,7 +246,6 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
   void SetAllowRunningOfInsecureContent(bool allowed);
   void DumpPermissionClientCallbacks();
   void DumpWindowStatusChanges();
-  void DumpProgressFinishedCallback();
   void DumpSpellCheckCallbacks();
   void DumpBackForwardList();
   void DumpSelectionRect();
@@ -505,8 +504,6 @@ gin::ObjectTemplateBuilder TestRunnerBindings::GetObjectTemplateBuilder(
                  &TestRunnerBindings::DumpPermissionClientCallbacks)
       .SetMethod("dumpWindowStatusChanges",
                  &TestRunnerBindings::DumpWindowStatusChanges)
-      .SetMethod("dumpProgressFinishedCallback",
-                 &TestRunnerBindings::DumpProgressFinishedCallback)
       .SetMethod("dumpSpellCheckCallbacks",
                  &TestRunnerBindings::DumpSpellCheckCallbacks)
       .SetMethod("dumpBackForwardList",
@@ -1211,11 +1208,6 @@ void TestRunnerBindings::DumpWindowStatusChanges() {
     runner_->DumpWindowStatusChanges();
 }
 
-void TestRunnerBindings::DumpProgressFinishedCallback() {
-  if (runner_)
-    runner_->DumpProgressFinishedCallback();
-}
-
 void TestRunnerBindings::DumpSpellCheckCallbacks() {
   if (runner_)
     runner_->DumpSpellCheckCallbacks();
@@ -1720,7 +1712,6 @@ void TestRunner::Reset() {
   dump_resource_request_callbacks_ = false;
   dump_resource_response_mime_types_ = false;
   dump_window_status_changes_ = false;
-  dump_progress_finished_callback_ = false;
   dump_spell_check_callbacks_ = false;
   dump_back_forward_list_ = false;
   dump_selection_rect_ = false;
@@ -1895,10 +1886,6 @@ bool TestRunner::shouldDumpStatusCallbacks() const {
   return dump_window_status_changes_;
 }
 
-bool TestRunner::shouldDumpProgressFinishedCallback() const {
-  return dump_progress_finished_callback_;
-}
-
 bool TestRunner::shouldDumpSpellCheckCallbacks() const {
   return dump_spell_check_callbacks_;
 }
@@ -2746,10 +2733,6 @@ void TestRunner::DumpWindowStatusChanges() {
   dump_window_status_changes_ = true;
 }
 
-void TestRunner::DumpProgressFinishedCallback() {
-  dump_progress_finished_callback_ = true;
-}
-
 void TestRunner::DumpSpellCheckCallbacks() {
   dump_spell_check_callbacks_ = true;
 }
diff --git a/components/test_runner/test_runner.h b/components/test_runner/test_runner.h
index 4a35f0e..1b03a30 100644
--- a/components/test_runner/test_runner.h
+++ b/components/test_runner/test_runner.h
@@ -105,7 +105,6 @@ class TestRunner : public WebTestRunner,
   bool shouldDumpResourceRequestCallbacks() const;
   bool shouldDumpResourceResponseMIMETypes() const;
   bool shouldDumpStatusCallbacks() const;
-  bool shouldDumpProgressFinishedCallback() const;
   bool shouldDumpSpellCheckCallbacks() const;
   bool shouldWaitUntilExternalURLLoad() const;
   const std::set<std::string>* httpHeadersToClear() const;
@@ -436,11 +435,6 @@ class TestRunner : public WebTestRunner,
   // It takes no arguments, and ignores any that may be present.
   void DumpWindowStatusChanges();
 
-  // This function sets a flag that tells the test_shell to print a line of
-  // descriptive text for the progress finished callback. It takes no
-  // arguments, and ignores any that may be present.
-  void DumpProgressFinishedCallback();
-
   // This function sets a flag that tells the test_shell to dump all
   // the lines of descriptive text about spellcheck execution.
   void DumpSpellCheckCallbacks();
@@ -770,10 +764,6 @@ class TestRunner : public WebTestRunner,
   // If true, the test_shell will dump all changes to window.status.
   bool dump_window_status_changes_;
 
-  // If true, the test_shell will output a descriptive line for the progress
-  // finished callback.
-  bool dump_progress_finished_callback_;
-
   // If true, the test_shell will output descriptive test for spellcheck
   // execution.
   bool dump_spell_check_callbacks_;
diff --git a/components/test_runner/web_frame_test_proxy.h b/components/test_runner/web_frame_test_proxy.h
index bfaf15c..00a287c4 100644
--- a/components/test_runner/web_frame_test_proxy.h
+++ b/components/test_runner/web_frame_test_proxy.h
@@ -268,11 +268,6 @@ class WebFrameTestProxy : public Base {
         source_frame, target_frame, target, event);
   }
 
-  virtual void didStopLoading() {
-    base_proxy_->DidStopLoading();
-    Base::didStopLoading();
-  }
-
   virtual void postAccessibilityEvent(const blink::WebAXObject& object,
                                       blink::WebAXEvent event) {
     base_proxy_->PostAccessibilityEvent(object, event);
diff --git a/components/test_runner/web_test_proxy.cc b/components/test_runner/web_test_proxy.cc
index be90a68..b9cde1e 100644
--- a/components/test_runner/web_test_proxy.cc
+++ b/components/test_runner/web_test_proxy.cc
@@ -971,11 +971,6 @@ void WebTestProxyBase::SetStatusText(const blink::WebString& text) {
       text.utf8().data() + "\n");
 }
 
-void WebTestProxyBase::DidStopLoading() {
-  if (test_interfaces_->GetTestRunner()->shouldDumpProgressFinishedCallback())
-    delegate_->PrintMessage("postProgressFinishedNotification\n");
-}
-
 void WebTestProxyBase::ShowContextMenu(
     const blink::WebContextMenuData& context_menu_data) {
   test_interfaces_->GetEventSender()->SetContextMenuData(context_menu_data);
diff --git a/components/test_runner/web_test_proxy.h b/components/test_runner/web_test_proxy.h
index 5c851a4..b43c3c6 100644
--- a/components/test_runner/web_test_proxy.h
+++ b/components/test_runner/web_test_proxy.h
@@ -165,7 +165,6 @@ class TEST_RUNNER_EXPORT WebTestProxyBase {
   blink::WebPlugin* CreatePlugin(blink::WebLocalFrame* frame,
                                  const blink::WebPluginParams& params);
   void SetStatusText(const blink::WebString& text);
-  void DidStopLoading();
   void ShowContextMenu(const blink::WebContextMenuData& data);
   blink::WebUserMediaClient* GetUserMediaClient();
   void PrintPage(blink::WebLocalFrame* frame);
diff --git a/components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java b/components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
index da02731..3349386 100644
--- a/components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
+++ b/components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
@@ -53,7 +53,7 @@ public class WebContentsDelegateAndroid {
     }
 
     @CalledByNative
-    public void onLoadStarted() {
+    public void onLoadStarted(boolean toDifferentDocument) {
     }
 
     @CalledByNative
diff --git a/components/web_contents_delegate_android/web_contents_delegate_android.cc b/components/web_contents_delegate_android/web_contents_delegate_android.cc
index 337044c..b535eb3 100644
--- a/components/web_contents_delegate_android/web_contents_delegate_android.cc
+++ b/components/web_contents_delegate_android/web_contents_delegate_android.cc
@@ -177,10 +177,14 @@ void WebContentsDelegateAndroid::LoadingStateChanged(WebContents* source,
     return;
   bool has_stopped = source == NULL || !source->IsLoading();
 
-  if (has_stopped)
+  if (has_stopped) {
     Java_WebContentsDelegateAndroid_onLoadStopped(env, obj.obj());
-  else
-    Java_WebContentsDelegateAndroid_onLoadStarted(env, obj.obj());
+  } else {
+    Java_WebContentsDelegateAndroid_onLoadStarted(
+        env,
+        obj.obj(),
+        to_different_document);
+  }
 }
 
 void WebContentsDelegateAndroid::LoadProgressChanged(WebContents* source,
diff --git a/third_party/WebKit/LayoutTests/http/tests/loading/progress-finished-callback-expected.txt b/third_party/WebKit/LayoutTests/http/tests/loading/progress-finished-callback-expected.txt
deleted file mode 100644
index 71ceefb..0000000
--- a/third_party/WebKit/LayoutTests/http/tests/loading/progress-finished-callback-expected.txt
+++ /dev/null
@@ -1,7 +0,0 @@
-main frame - didStartProvisionalLoadForFrame
-main frame - didCommitLoadForFrame
-main frame - didFinishDocumentLoadForFrame
-main frame - didHandleOnloadEventsForFrame
-postProgressFinishedNotification
-main frame - didFinishLoadForFrame
-This is a test of progress finished callback ordering in relation to frame loader callbacks. It is only useful inside the regression test tool.
diff --git a/third_party/WebKit/LayoutTests/http/tests/loading/progress-finished-callback.html b/third_party/WebKit/LayoutTests/http/tests/loading/progress-finished-callback.html
deleted file mode 100644
index 7cef671..0000000
--- a/third_party/WebKit/LayoutTests/http/tests/loading/progress-finished-callback.html
+++ /dev/null
@@ -1,7 +0,0 @@
-<script>
-if (window.testRunner) {
-    testRunner.dumpAsText();
-    testRunner.dumpProgressFinishedCallback();
-}
-</script>
-This is a test of progress finished callback ordering in relation to frame loader callbacks. It is only useful inside the regression test tool.
diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
index cb3e39a1..01b2985 100644
--- a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
@@ -292,19 +292,10 @@ void DocumentLoader::finishedLoading(double finishTime)
             commitData(0, 0);
     }
 
-    endWriting(m_writer.get());
-
-    if (!m_mainDocumentError.isNull())
-        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())
-            memoryCache()->remove(m_mainResource.get());
-    }
     m_applicationCacheHost->finishedLoadingMainResource();
+    endWriting(m_writer.get());
+    if (m_state < MainResourceDone)
+        m_state = MainResourceDone;
     clearMainResourceHandle();
 }
 
diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.h b/third_party/WebKit/Source/core/loader/DocumentLoader.h
index c5c1755..163a511 100644
--- a/third_party/WebKit/Source/core/loader/DocumentLoader.h
+++ b/third_party/WebKit/Source/core/loader/DocumentLoader.h
@@ -105,6 +105,10 @@ public:
     void setReplacesCurrentHistoryItem(bool replacesCurrentHistoryItem) { m_replacesCurrentHistoryItem = replacesCurrentHistoryItem; }
 
     bool isCommittedButEmpty() const { return m_state == Committed; }
+
+    void setSentDidFinishLoad() { m_state = SentDidFinishLoad; }
+    bool sentDidFinishLoad() const { return m_state == SentDidFinishLoad; }
+
     NavigationType navigationType() const { return m_navigationType; }
     void setNavigationType(NavigationType navigationType) { m_navigationType = navigationType; }
 
@@ -232,7 +236,8 @@ private:
         Provisional,
         Committed,
         DataReceived,
-        MainResourceDone
+        MainResourceDone,
+        SentDidFinishLoad
     };
     State m_state;
 
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index 1f8c9f4..0f25b1f 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -525,37 +525,34 @@ static bool shouldComplete(Document* document)
     return allDescendantsAreComplete(document->frame());
 }
 
-static bool shouldSendCompleteNotifications(LocalFrame* frame)
+static bool shouldSendFinishNotification(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;
 
-    // FIXME: We might have already sent stop notifications and be re-completing.
-    if (!frame->isLoading())
+    // Don't send didFinishLoad more than once per DocumentLoader.
+    if (frame->loader().documentLoader()->sentDidFinishLoad())
         return false;
 
-    // The readystatechanged or load event may have disconnected this frame.
-    if (!frame->client())
+    // We might have declined to run the load event due to an imminent content-initiated navigation.
+    if (!frame->document()->loadEventFinished())
         return false;
 
     // An event might have restarted a child frame.
     if (!allDescendantsAreComplete(frame))
         return false;
+    return true;
+}
 
-    // An event might have restarted this frame by scheduling a new navigation.
-    if (frame->loader().provisionalDocumentLoader())
-        return false;
-
-    // A navigation is still scheduled in the embedder, so don't complete yet.
-    if (frame->loader().client()->hasPendingNavigation())
-        return false;
-
-    // We might have declined to run the load event due to an imminent content-initiated navigation.
-    if (!frame->document()->loadEventFinished())
+static bool shouldSendCompleteNotification(LocalFrame* frame)
+{
+    // FIXME: We might have already sent stop notifications and be re-completing.
+    if (!frame->isLoading())
         return false;
-
-    return true;
+    // Only send didStopLoading() if there are no navigations in progress at all,
+    // whether committed, provisional, or pending.
+    return frame->loader().documentLoader()->sentDidFinishLoad() && !frame->loader().provisionalDocumentLoader() && !frame->loader().client()->hasPendingNavigation();
 }
 
 void FrameLoader::checkCompleted()
@@ -574,7 +571,22 @@ void FrameLoader::checkCompleted()
     if (m_frame->view())
         m_frame->view()->handleLoadCompleted();
 
-    if (shouldSendCompleteNotifications(m_frame)) {
+    // The readystatechanged or load event may have disconnected this frame.
+    if (!m_frame->client())
+        return;
+
+    if (shouldSendFinishNotification(m_frame)) {
+        // Report mobile vs. desktop page statistics. This will only report on Android.
+        if (m_frame->isMainFrame())
+            m_frame->document()->viewportDescription().reportMobilePageStats(m_frame);
+        m_documentLoader->setSentDidFinishLoad();
+        client()->dispatchDidFinishLoad();
+        // Finishing the load can detach the frame when running layout tests.
+        if (!m_frame->client())
+            return;
+    }
+
+    if (shouldSendCompleteNotification(m_frame)) {
         m_progressTracker->progressCompleted();
         // Retry restoring scroll offset since finishing loading disables content
         // size clamping.
@@ -582,11 +594,6 @@ void FrameLoader::checkCompleted()
 
         m_loadType = FrameLoadTypeStandard;
         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();
@@ -1220,6 +1227,7 @@ void FrameLoader::receivedMainResourceError(DocumentLoader* loader, const Resour
         ASSERT(loader == m_documentLoader);
         if (m_frame->document()->parser())
             m_frame->document()->parser()->stopParsing();
+        m_documentLoader->setSentDidFinishLoad();
         if (!m_provisionalDocumentLoader && m_frame->isLoading()) {
             client()->dispatchDidFailLoad(error, historyCommitType);
             m_progressTracker->progressCompleted();
diff --git a/third_party/WebKit/Source/web/WebFrame.cpp b/third_party/WebKit/Source/web/WebFrame.cpp
index f4cde05..5479891 100644
--- a/third_party/WebKit/Source/web/WebFrame.cpp
+++ b/third_party/WebKit/Source/web/WebFrame.cpp
@@ -282,6 +282,13 @@ WebFrame* WebFrame::fromFrameOwnerElement(const WebElement& webElement)
     return fromFrame(toHTMLFrameElementBase(element)->contentFrame());
 }
 
+bool WebFrame::isLoading() const
+{
+    if (Frame* frame = toCoreFrame(this))
+        return frame->isLoading();
+    return false;
+}
+
 WebFrame* WebFrame::fromFrame(Frame* frame)
 {
     if (!frame)
diff --git a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
index 8e5b501..7ec0ced 100644
--- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
+++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
@@ -8297,4 +8297,36 @@ TEST_F(WebFrameTest, ImageDocumentLoadFinishTime)
     EXPECT_EQ(loader->timing().responseEnd(), resource->loadFinishTime());
 }
 
+class CallbackOrderingWebFrameClient : public FrameTestHelpers::TestWebFrameClient {
+public:
+    CallbackOrderingWebFrameClient() : m_callbackCount(0) { }
+
+    void didStartLoading(bool toDifferentDocument) override
+    {
+        EXPECT_EQ(0, m_callbackCount++);
+        FrameTestHelpers::TestWebFrameClient::didStartLoading(toDifferentDocument);
+    }
+    void didStartProvisionalLoad(WebLocalFrame*, double) override { EXPECT_EQ(1, m_callbackCount++); }
+    void didCommitProvisionalLoad(WebLocalFrame*, const WebHistoryItem&, WebHistoryCommitType) override { EXPECT_EQ(2, m_callbackCount++); }
+    void didFinishDocumentLoad(WebLocalFrame*, bool documentIsEmpty) override { EXPECT_EQ(3, m_callbackCount++); }
+    void didHandleOnloadEvents(WebLocalFrame*) override { EXPECT_EQ(4, m_callbackCount++); }
+    void didFinishLoad(WebLocalFrame*) override { EXPECT_EQ(5, m_callbackCount++); }
+    void didStopLoading() override
+    {
+        EXPECT_EQ(6, m_callbackCount++);
+        FrameTestHelpers::TestWebFrameClient::didStopLoading();
+    }
+
+private:
+    int m_callbackCount;
+};
+
+TEST_F(WebFrameTest, CallbackOrdering)
+{
+    registerMockedHttpURLLoad("foo.html");
+    FrameTestHelpers::WebViewHelper webViewHelper;
+    CallbackOrderingWebFrameClient client;
+    webViewHelper.initializeAndLoad(m_baseURL + "foo.html", true, &client);
+}
+
 } // namespace blink
diff --git a/third_party/WebKit/public/web/WebFrame.h b/third_party/WebKit/public/web/WebFrame.h
index 56c37aa..1b62231 100644
--- a/third_party/WebKit/public/web/WebFrame.h
+++ b/third_party/WebKit/public/web/WebFrame.h
@@ -425,6 +425,9 @@ public:
     // Returns the number of registered unload listeners.
     virtual unsigned unloadListenerCount() const = 0;
 
+    // Will return true if between didStartLoading and didStopLoading notifications.
+    virtual bool isLoading() const;
+
 
     // Editing -------------------------------------------------------------
 
diff --git a/tools/perf/measurements/v8_detached_context_age_in_gc_unittest.py b/tools/perf/measurements/v8_detached_context_age_in_gc_unittest.py
index 1d9151ac..eded79a9 100644
--- a/tools/perf/measurements/v8_detached_context_age_in_gc_unittest.py
+++ b/tools/perf/measurements/v8_detached_context_age_in_gc_unittest.py
@@ -91,7 +91,7 @@ class V8DetachedContextAgeInGCTests(page_test_test_case.PageTestTestCase):
     page_set.AddStory(page)
     metric = v8_detached_context_age_in_gc.V8DetachedContextAgeInGC()
     results = self.RunMeasurement(metric, page_set, options=self._options)
-    self.assertEquals(0, len(results.failures))
+    self.assertEquals(0, len(results.failures), msg=str(results.failures))
     actual = _ActualValues(results)['V8_DetachedContextAgeInGC']
     self.assertLessEqual(0, actual.value)
     self.assertEqual('garbage_collections', actual.units)