[PM] Use network and CPU quiescence for PageNode::IsLoading().

With this CL, PageNode::IsLoading() transitions to true when data is
received for a top-level navigation to a different document. It
transitions to false when the page reaches an "almost idle" state,
based on CPU and network quiescence, or after an absolute timeout.

This definition of "loading" is considered better to drive policies,
as it indicates when the page stops doing work after a navigation,
rather than when the page is loading resources (it may still have a
lot of Javascript to run to present useful content to the user).

Change-Id: I4fc5b277933133229fde7585d6710f63e6b8244d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015820
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738272}

chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.cc

@@ -24,6 +24,7 @@
 #include "components/performance_manager/embedder/performance_manager_lifetime.h"
 #include "components/performance_manager/embedder/performance_manager_registry.h"
 #include "components/performance_manager/performance_manager_lock_observer.h"
+#include "components/performance_manager/public/decorators/page_load_tracker_decorator_helper.h"
 #include "components/performance_manager/public/graph/graph.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_features.h"
@@ -127,6 +128,8 @@ void ChromeBrowserMainExtraPartsPerformanceManager::PostCreateThreads() {
 
   page_live_state_data_helper_ =
       std::make_unique<performance_manager::PageLiveStateDecoratorHelper>();
+  page_load_tracker_decorator_helper_ =
+      std::make_unique<performance_manager::PageLoadTrackerDecoratorHelper>();
 }
 
 void ChromeBrowserMainExtraPartsPerformanceManager::PostMainMessageLoopRun() {
@@ -138,6 +141,7 @@ void ChromeBrowserMainExtraPartsPerformanceManager::PostMainMessageLoopRun() {
   g_browser_process->profile_manager()->RemoveObserver(this);
   observed_profiles_.RemoveAll();
 
+  page_load_tracker_decorator_helper_.reset();
   page_live_state_data_helper_.reset();
 
   // There may still be WebContents and RenderProcessHosts with attached user

chrome/browser/performance_manager/chrome_browser_main_extra_parts_performance_manager.h

@@ -26,6 +26,7 @@ namespace performance_manager {
 class BrowserChildProcessWatcher;
 class Graph;
 class PageLiveStateDecoratorHelper;
+class PageLoadTrackerDecoratorHelper;
 class PerformanceManager;
 class PerformanceManagerRegistry;
 }  // namespace performance_manager
@@ -76,10 +77,14 @@ class ChromeBrowserMainExtraPartsPerformanceManager
 
   ScopedObserver<Profile, ProfileObserver> observed_profiles_{this};
 
-  // Needed to properly maintain some of the PageLiveStateDecorator' properties.
+  // Needed to maintain some of the PageLiveStateDecorator' properties.
   std::unique_ptr<performance_manager::PageLiveStateDecoratorHelper>
       page_live_state_data_helper_;
 
+  // Needed to maintain the PageNode::IsLoading() property.
+  std::unique_ptr<performance_manager::PageLoadTrackerDecoratorHelper>
+      page_load_tracker_decorator_helper_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainExtraPartsPerformanceManager);
 };
 

chrome/browser/performance_manager/page_load_tracker_decorator_browsertest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/performance_manager/graph/page_node_impl.h"
+#include "components/performance_manager/performance_manager_impl.h"
+#include "components/performance_manager/public/performance_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace performance_manager {
+
+namespace {
+
+// A class that waits for the IsLoading() property of a PageNode to transition
+// to a desired value. Generates an error if the first IsLoading() transitions
+// is not for the observed PageNode. Ignores IsLoading() transitions after the
+// first one.
+class PageIsLoadingObserver : public PageNode::ObserverDefaultImpl,
+                              public GraphOwnedDefaultImpl {
+ public:
+  PageIsLoadingObserver(base::WeakPtr<PageNode> page_node,
+                        bool desired_is_loading)
+      : page_node_(page_node), desired_is_loading_(desired_is_loading) {
+    auto* perf_manager = PerformanceManagerImpl::GetInstance();
+    DCHECK(perf_manager);
+    perf_manager->CallOnGraphImpl(
+        FROM_HERE,
+        base::BindLambdaForTesting([&](performance_manager::GraphImpl* graph) {
+          EXPECT_TRUE(page_node_);
+
+          if (page_node_->IsLoading() == desired_is_loading_) {
+            run_loop_.Quit();
+          } else {
+            graph_ = graph;
+            graph_->AddPageNodeObserver(this);
+          }
+        }));
+  }
+
+  ~PageIsLoadingObserver() override = default;
+
+  void Wait() {
+    // The RunLoop is quit when |page_node_->IsLoading()| becomes equal to
+    // |desired_is_loading_|.
+    run_loop_.Run();
+  }
+
+ private:
+  // PageNodeObserver:
+  void OnIsLoadingChanged(const PageNode* page_node) override {
+    EXPECT_EQ(page_node_.get(), page_node);
+    EXPECT_EQ(page_node->IsLoading(), desired_is_loading_);
+    graph_->RemovePageNodeObserver(this);
+    run_loop_.Quit();
+  }
+
+  // This RunLoop is quit when |page_node_->IsLoading()| is equal to
+  // |desired_is_loading_|.
+  base::RunLoop run_loop_;
+
+  // The watched PageNode.
+  const base::WeakPtr<PageNode> page_node_;
+
+  // Desired value for |page_node_->IsLoading()|.
+  const bool desired_is_loading_;
+
+  // Set when registering |this| as a PageNodeObserver. Used to unregister.
+  GraphImpl* graph_ = nullptr;
+};
+
+}  // namespace
+
+class PageLoadTrackerDecoratorTest : public InProcessBrowserTest {
+ public:
+  PageLoadTrackerDecoratorTest() = default;
+  ~PageLoadTrackerDecoratorTest() override = default;
+};
+
+// Integration test verifying that everything is hooked up in Chrome to update
+// PageNode::IsLoading() is updated on navigation. See
+// PageLoadTrackerDecoratorTest for low level unit tests.
+IN_PROC_BROWSER_TEST_F(PageLoadTrackerDecoratorTest, PageNodeIsLoading) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  base::WeakPtr<PageNode> page_node =
+      PerformanceManager::GetPageNodeForWebContents(
+          browser()->tab_strip_model()->GetActiveWebContents());
+
+  // Wait until IsLoading() is false (the initial navigation may or may not be
+  // ongoing).
+  PageIsLoadingObserver observer1(page_node, false);
+  observer1.Wait();
+
+  // Create an Observer that will observe IsLoading() becoming true when the
+  // navigation below starts.
+  PageIsLoadingObserver observer2(page_node, true);
+
+  // Navigate.
+  browser()->OpenURL(content::OpenURLParams(
+      embedded_test_server()->GetURL("/empty.html"), content::Referrer(),
+      WindowOpenDisposition::CURRENT_TAB, ui::PAGE_TRANSITION_TYPED, false));
+
+  // Wait until IsLoading() is true.
+  observer2.Wait();
+
+  // Wait until IsLoading() is false.
+  PageIsLoadingObserver observer3(page_node, false);
+  observer3.Wait();
+}
+
+}  // namespace performance_manager

chrome/browser/resource_coordinator/tab_helper.cc

@@ -74,19 +74,12 @@ bool ResourceCoordinatorTabHelper::IsFrozen(content::WebContents* contents) {
   return false;
 }
 
-void ResourceCoordinatorTabHelper::DidStartLoading() {
-  TabLoadTracker::Get()->DidStartLoading(web_contents());
-}
-
 void ResourceCoordinatorTabHelper::DidReceiveResponse() {
   TabLoadTracker::Get()->DidReceiveResponse(web_contents());
 }
 
-void ResourceCoordinatorTabHelper::DidFailLoad(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    int error_code) {
-  TabLoadTracker::Get()->DidFailLoad(web_contents());
+void ResourceCoordinatorTabHelper::DidStopLoading() {
+  TabLoadTracker::Get()->DidStopLoading(web_contents());
 }
 
 void ResourceCoordinatorTabHelper::RenderProcessGone(

chrome/browser/resource_coordinator/tab_helper.h

@@ -38,11 +38,8 @@ class ResourceCoordinatorTabHelper
   static bool IsFrozen(content::WebContents* contents);
 
   // WebContentsObserver overrides.
-  void DidStartLoading() override;
   void DidReceiveResponse() override;
-  void DidFailLoad(content::RenderFrameHost* render_frame_host,
-                   const GURL& validated_url,
-                   int error_code) override;
+  void DidStopLoading() override;
   void RenderProcessGone(base::TerminationStatus status) override;
   void WebContentsDestroyed() override;
   void DidFinishNavigation(

chrome/browser/resource_coordinator/tab_load_tracker.cc

@@ -112,7 +112,7 @@ void TabLoadTracker::TransitionStateForTesting(
     LoadingState loading_state) {
   auto it = tabs_.find(web_contents);
   DCHECK(it != tabs_.end());
-  TransitionState(it, loading_state, false);
+  TransitionState(it, loading_state);
 }
 
 TabLoadTracker::TabLoadTracker() = default;
@@ -127,8 +127,6 @@ void TabLoadTracker::StartTracking(content::WebContents* web_contents) {
   // documented in TransitionState.
   WebContentsData data;
   data.loading_state = loading_state;
-  if (data.loading_state == LOADING)
-    data.did_start_loading_seen = true;
   data.is_ui_tab = IsUiTab(web_contents);
   tabs_.insert(std::make_pair(web_contents, data));
   ++state_counts_[static_cast<size_t>(data.loading_state)];
@@ -159,39 +157,29 @@ void TabLoadTracker::StopTracking(content::WebContents* web_contents) {
     observer.OnStopTracking(web_contents, loading_state);
 }
 
-void TabLoadTracker::DidStartLoading(content::WebContents* web_contents) {
+void TabLoadTracker::DidReceiveResponse(content::WebContents* web_contents) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  // Only observe top-level navigation to a different document.
   if (!web_contents->IsLoadingToDifferentDocument())
     return;
+
   auto it = tabs_.find(web_contents);
   DCHECK(it != tabs_.end());
-  if (it->second.loading_state == LOADING) {
-    DCHECK(it->second.did_start_loading_seen);
-    return;
-  }
-  it->second.did_start_loading_seen = true;
+  TransitionState(it, LOADING);
 }
 
-void TabLoadTracker::DidReceiveResponse(content::WebContents* web_contents) {
+void TabLoadTracker::DidStopLoading(content::WebContents* web_contents) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto it = tabs_.find(web_contents);
   DCHECK(it != tabs_.end());
-  if (it->second.loading_state == LOADING) {
-    DCHECK(it->second.did_start_loading_seen);
-    return;
-  }
-  // A transition to loading requires both DidStartLoading (navigation
-  // committed) and DidReceiveResponse (data has been trasmitted over the
-  // network) events to occur. This is because NavigationThrottles can block
-  // actual network requests, but not the rest of the state machinery.
-  if (!it->second.did_start_loading_seen)
-    return;
-  TransitionState(it, LOADING, true);
-}
 
-void TabLoadTracker::DidFailLoad(content::WebContents* web_contents) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  MaybeTransitionToLoaded(web_contents);
+  // Corner case: An unloaded tab that starts loading but never receives a
+  // response transitions to the LOADED state when loading stops, without
+  // traversing the LOADING state. This can happen when the server doesn't
+  // respond or when there is no network connection.
+  if (it->second.loading_state == LoadingState::UNLOADED)
+    TransitionState(it, LOADED);
 }
 
 void TabLoadTracker::RenderProcessGone(content::WebContents* web_contents,
@@ -216,19 +204,19 @@ void TabLoadTracker::RenderProcessGone(content::WebContents* web_contents,
   // can happen if the renderer crashes between the UNLOADED and LOADING states.
   if (it->second.loading_state == UNLOADED)
     return;
-  TransitionState(it, UNLOADED, true);
+  TransitionState(it, UNLOADED);
 }
 
-void TabLoadTracker::OnPageAlmostIdle(content::WebContents* web_contents) {
+void TabLoadTracker::OnPageStoppedLoading(content::WebContents* web_contents) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // TabManager::ResourceCoordinatorSignalObserver filters late notifications
-  // so here we can assume the event pertains to a live web_contents and
-  // its most recent navigation. However, the graph tracks contents that aren't
-  // tracked by this object.
-  if (!base::Contains(tabs_, web_contents))
+
+  auto it = tabs_.find(web_contents);
+  if (it == tabs_.end()) {
+    // The graph tracks objects that are not tracked by this object.
     return;
+  }
 
-  MaybeTransitionToLoaded(web_contents);
+  TransitionState(it, LOADED);
 }
 
 TabLoadTracker::LoadingState TabLoadTracker::DetermineLoadingState(
@@ -254,43 +242,14 @@ TabLoadTracker::LoadingState TabLoadTracker::DetermineLoadingState(
   return loading_state;
 }
 
-void TabLoadTracker::MaybeTransitionToLoaded(
-    content::WebContents* web_contents) {
-  auto it = tabs_.find(web_contents);
-  DCHECK(it != tabs_.end());
-  if (it->second.loading_state != LOADING)
-    return;
-  TransitionState(it, LOADED, true);
-}
-
 void TabLoadTracker::TransitionState(TabMap::iterator it,
-                                     LoadingState loading_state,
-                                     bool validate_transition) {
-#if DCHECK_IS_ON()
-  if (validate_transition) {
-    // Validate the transition.
-    switch (loading_state) {
-      case LOADING: {
-        DCHECK_NE(LOADING, it->second.loading_state);
-        DCHECK(it->second.did_start_loading_seen);
-        break;
-      }
-
-      case LOADED: {
-        DCHECK_EQ(LOADING, it->second.loading_state);
-        DCHECK(it->second.did_start_loading_seen);
-        break;
-      }
-
-      case UNLOADED: {
-        DCHECK_NE(UNLOADED, it->second.loading_state);
-        break;
-      }
-    }
+                                     LoadingState loading_state) {
+  LoadingState previous_state = it->second.loading_state;
+
+  if (previous_state == loading_state) {
+    return;
   }
-#endif
 
-  LoadingState previous_state = it->second.loading_state;
   --state_counts_[static_cast<size_t>(previous_state)];
   it->second.loading_state = loading_state;
   ++state_counts_[static_cast<size_t>(loading_state)];
@@ -300,11 +259,6 @@ void TabLoadTracker::TransitionState(TabMap::iterator it,
     --ui_tab_state_counts_[static_cast<size_t>(previous_state)];
   }
 
-  // If the destination state is LOADED, then also clear the
-  // |did_start_loading_seen| state.
-  if (loading_state == LOADED)
-    it->second.did_start_loading_seen = false;
-
   // Store |it->first| instead of passing it directly in the loop below in case
   // an observer starts/stops tracking a WebContents and invalidates |it|.
   content::WebContents* web_contents = it->first;

chrome/browser/resource_coordinator/tab_load_tracker.h

@@ -25,15 +25,17 @@ class ResourceCoordinatorParts;
 class ResourceCoordinatorTabHelper;
 class TabManagerResourceCoordinatorSignalObserverHelper;
 
+// DEPRECATED. New users must observe PageNode::IsLoading() with a
+// PageNodeObserver. For guidance: //components/performance_manager/OWNERS
+//
 // This class has the sole purpose of tracking the state of all tab-related
 // WebContents, and whether or not they are in an unloaded, currently loading,
 // or loaded state.
 //
-// This class must be bound to a given Sequence and all access to it must
-// occur on that Sequence. In practice, this is intended to be on the UI
-// thread as the notifications of interest occur natively on that thread. All
-// calculations are very short and quick, so it is suitable for use on that
-// thread.
+// This class must be bound to a given Sequence and all access to it must occur
+// on that Sequence. In practice, this is intended to be on the UI thread as the
+// notifications of interest occur natively on that thread. All calculations are
+// very short and quick, so it is suitable for use on that thread.
 //
 // This class is intended to be created in early startup and persists as a
 // singleton in the browser process. It is deliberately leaked at shutdown.
@@ -55,22 +57,17 @@ class TabLoadTracker {
   // context of a WebContents:
   //
   // An initially constructed WebContents with no loaded content is UNLOADED.
-  // A WebContents that started loading but that errored out before receiving
-  // sufficient content to render is also considered UNLOADED. Can only
-  // transition from UNLOADED to LOADING.
   //
-  // A WebContents with an ongoing main-frame navigation (to a new document)
-  // is in a loading state. More precisely, it is considered loading once
-  // network data has started to be transmitted, and not simply when the
-  // navigation has started. This considers throttled navigations as not yet
-  // loading, and will only transition to loading once the throttle has been
-  // removed. Can transition from LOADING to UNLOADED or LOADED.
+  // A WebContents transitions to LOADING when network data starts being
+  // received for a top-level load to a different document. This considers
+  // throttled navigations as not yet loading, and will only transition to
+  // loading once the throttle has been removed.
+  //
+  // A LOADING WebContents transitions to LOADED when it reaches an "almost
+  // idle" state, based on CPU and network quiescence or after an absolute
+  // timeout (see PageLoadTrackerDecorator).
   //
-  // A WebContents with a committed navigation whose PageAlmostIdle event or
-  // DidFailLoad event has fired is no longer considered to be LOADING. If any
-  // content has been rendered prior to the failure the document is considered
-  // LOADED, otherwise it is considered UNLOADED. Can transition from LOADED to
-  // LOADING.
+  // A WebContents transitions to UNLOADED when its render process is gone.
 
   ~TabLoadTracker();
 
@@ -143,17 +140,19 @@ class TabLoadTracker {
   void StopTracking(content::WebContents* web_contents);
 
   // These are analogs of WebContentsObserver functions. This class is not
-  // actually an observer, but the relevant events are forwarded to it from
-  // the TabManager.
-  void DidStartLoading(content::WebContents* web_contents);
+  // actually an observer, but the relevant events are forwarded to it from the
+  // TabManager.
+  //
+  // In all cases, a call to DidReceiveResponse() is expected to be followed by
+  // a call to StopTracking(), RenderProcessGone() or OnPageStoppedLoading().
   void DidReceiveResponse(content::WebContents* web_contents);
-  void DidFailLoad(content::WebContents* web_contents);
+  void DidStopLoading(content::WebContents* web_contents);
   void RenderProcessGone(content::WebContents* web_contents,
                          base::TerminationStatus status);
 
   // Notifications to this are driven by the
   // TabManager::ResourceCoordinatorSignalObserver.
-  void OnPageAlmostIdle(content::WebContents* web_contents);
+  void OnPageStoppedLoading(content::WebContents* web_contents);
 
   // Returns true if |web_contents| is a UI tab and false otherwise. This is
   // used to filter out cases where tab helpers are attached to a non-UI tab
@@ -170,7 +169,6 @@ class TabLoadTracker {
   // Some metadata used to track the current state of the WebContents.
   struct WebContentsData {
     LoadingState loading_state = LoadingState::UNLOADED;
-    bool did_start_loading_seen = false;
     bool is_ui_tab = false;
   };
 
@@ -179,17 +177,11 @@ class TabLoadTracker {
   // Helper function for determining the current state of a |web_contents|.
   LoadingState DetermineLoadingState(content::WebContents* web_contents);
 
-  // Helper function for marking a load as finished, if possible. If the tab
-  // isn't currently marked as loading then this does nothing.
-  void MaybeTransitionToLoaded(content::WebContents* web_contents);
-
   // Transitions a web contents to the given state. This updates the various
   // |state_counts_| and |tabs_| data. Setting |validate_transition| to false
   // means that valid state machine transitions aren't enforced via checks; this
   // is only used by state transitions forced via TransitionStateForTesting.
-  void TransitionState(TabMap::iterator it,
-                       LoadingState loading_state,
-                       bool validate_transition);
+  void TransitionState(TabMap::iterator it, LoadingState loading_state);
 
   // The list of known WebContents and their states. This includes both UI and
   // non-UI tabs.

chrome/browser/resource_coordinator/tab_load_tracker_unittest.cc

@@ -51,14 +51,12 @@ std::unique_ptr<content::NavigationSimulator> NavigateAndKeepLoading(
 // Test wrapper of TabLoadTracker that exposes some internals.
 class TestTabLoadTracker : public TabLoadTracker {
  public:
-  using TabLoadTracker::StartTracking;
-  using TabLoadTracker::StopTracking;
-  using TabLoadTracker::DidStartLoading;
+  using TabLoadTracker::DetermineLoadingState;
   using TabLoadTracker::DidReceiveResponse;
-  using TabLoadTracker::DidFailLoad;
+  using TabLoadTracker::OnPageStoppedLoading;
   using TabLoadTracker::RenderProcessGone;
-  using TabLoadTracker::OnPageAlmostIdle;
-  using TabLoadTracker::DetermineLoadingState;
+  using TabLoadTracker::StartTracking;
+  using TabLoadTracker::StopTracking;
 
   TestTabLoadTracker() : all_tabs_are_non_ui_tabs_(false) {}
   virtual ~TestTabLoadTracker() {}
@@ -66,15 +64,6 @@ class TestTabLoadTracker : public TabLoadTracker {
   // Some accessors for TabLoadTracker internals.
   const TabMap& tabs() const { return tabs_; }
 
-  // Determines if the tab has been marked as having received the
-  // DidStartLoading event.
-  bool DidStartLoadingSeen(content::WebContents* web_contents) {
-    auto it = tabs_.find(web_contents);
-    if (it == tabs_.end())
-      return false;
-    return it->second.did_start_loading_seen;
-  }
-
   bool IsUiTab(content::WebContents* web_contents) override {
     if (all_tabs_are_non_ui_tabs_)
       return false;
@@ -117,15 +106,9 @@ class TestWebContentsObserver : public content::WebContentsObserver {
   ~TestWebContentsObserver() override {}
 
   // content::WebContentsObserver:
-  void DidStartLoading() override { tracker_->DidStartLoading(web_contents()); }
   void DidReceiveResponse() override {
     tracker_->DidReceiveResponse(web_contents());
   }
-  void DidFailLoad(content::RenderFrameHost* render_frame_host,
-                   const GURL& validated_url,
-                   int error_code) override {
-    tracker_->DidFailLoad(web_contents());
-  }
   void RenderProcessGone(base::TerminationStatus status) override {
     tracker_->RenderProcessGone(web_contents(), status);
   }
@@ -290,7 +273,7 @@ void TabLoadTrackerTest::StateTransitionsTest(bool use_non_ui_tabs) {
   } else {
     EXPECT_TAB_AND_UI_TAB_COUNTS(3, 1, 1, 1);
   }
-  tracker().OnPageAlmostIdle(contents2());
+  tracker().OnPageStoppedLoading(contents2());
 
   if (use_non_ui_tabs) {
     EXPECT_TAB_COUNTS(3, 1, 0, 2);
@@ -314,25 +297,10 @@ void TabLoadTrackerTest::StateTransitionsTest(bool use_non_ui_tabs) {
   }
   testing::Mock::VerifyAndClearExpectations(&observer());
 
-  // Stop the loading with an error. The tab should go back to a LOADED
-  // state.
-  EXPECT_CALL(observer(),
-              OnLoadingStateChange(contents1(), LoadingState::LOADING,
-                                   LoadingState::LOADED));
-  navigation_tab_1->FailLoading(GURL("http://baz.com"), 500);
-  testing::Mock::VerifyAndClearExpectations(&observer());
-  if (use_non_ui_tabs) {
-    EXPECT_TAB_COUNTS(3, 0, 0, 3);
-    EXPECT_UI_TAB_COUNTS(0, 0, 0, 0);
-  } else {
-    EXPECT_TAB_AND_UI_TAB_COUNTS(3, 0, 0, 3);
-  }
-  testing::Mock::VerifyAndClearExpectations(&observer());
-
   // Crash the render process corresponding to the main frame of a tab. This
   // should cause the tab to transition to the UNLOADED state.
   EXPECT_CALL(observer(),
-              OnLoadingStateChange(contents1(), LoadingState::LOADED,
+              OnLoadingStateChange(contents1(), LoadingState::LOADING,
                                    LoadingState::UNLOADED));
   content::MockRenderProcessHost* rph =
       static_cast<content::MockRenderProcessHost*>(

chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.cc

@@ -23,9 +23,8 @@ namespace resource_coordinator {
 // and can't be forward declared.
 class TabManagerResourceCoordinatorSignalObserverHelper {
  public:
-  static void OnPageAlmostIdle(content::WebContents* web_contents) {
-    // This object is create on demand, so always exists.
-    TabLoadTracker::Get()->OnPageAlmostIdle(web_contents);
+  static void OnPageStoppedLoading(content::WebContents* web_contents) {
+    TabLoadTracker::Get()->OnPageStoppedLoading(web_contents);
   }
 };
 
@@ -37,16 +36,14 @@ TabManager::ResourceCoordinatorSignalObserver::
 TabManager::ResourceCoordinatorSignalObserver::
     ~ResourceCoordinatorSignalObserver() = default;
 
-void TabManager::ResourceCoordinatorSignalObserver::OnPageAlmostIdleChanged(
+void TabManager::ResourceCoordinatorSignalObserver::OnIsLoadingChanged(
     const PageNode* page_node) {
-  // Only notify of changes to almost idle.
-  if (!page_node->IsPageAlmostIdle())
-    return;
-  // Forward the notification over to the UI thread.
-  base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                 base::BindOnce(&OnPageAlmostIdleOnUi, tab_manager_,
-                                page_node->GetContentsProxy(),
-                                page_node->GetNavigationID()));
+  // Forward the notification over to the UI thread when the page stops loading.
+  if (!page_node->IsLoading()) {
+    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
+                   base::BindOnce(&OnPageStoppedLoadingOnUi,
+                                  page_node->GetContentsProxy()));
+  }
 }
 
 void TabManager::ResourceCoordinatorSignalObserver::
@@ -94,14 +91,11 @@ TabManager::ResourceCoordinatorSignalObserver::GetContentsForDispatch(
 }
 
 // static
-void TabManager::ResourceCoordinatorSignalObserver::OnPageAlmostIdleOnUi(
-    const base::WeakPtr<TabManager>& tab_manager,
-    const WebContentsProxy& contents_proxy,
-    int64_t navigation_id) {
+void TabManager::ResourceCoordinatorSignalObserver::OnPageStoppedLoadingOnUi(
+    const WebContentsProxy& contents_proxy) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (auto* contents =
-          GetContentsForDispatch(tab_manager, contents_proxy, navigation_id)) {
-    TabManagerResourceCoordinatorSignalObserverHelper::OnPageAlmostIdle(
+  if (auto* contents = contents_proxy.Get()) {
+    TabManagerResourceCoordinatorSignalObserverHelper::OnPageStoppedLoading(
         contents);
   }
 }

chrome/browser/resource_coordinator/tab_manager_resource_coordinator_signal_observer.h

@@ -39,7 +39,7 @@ class TabManager::ResourceCoordinatorSignalObserver
 
   // PageNode::ObserverDefaultImpl:
   // This function run on the performance manager sequence.
-  void OnPageAlmostIdleChanged(const PageNode* page_node) override;
+  void OnIsLoadingChanged(const PageNode* page_node) override;
 
   // GraphOwned implementation:
   void OnPassedToGraph(Graph* graph) override;
@@ -55,11 +55,8 @@ class TabManager::ResourceCoordinatorSignalObserver
       const WebContentsProxy& contents_proxy,
       int64_t navigation_id);
 
-  // Equivalent to the the GraphObserver functions above, but these are the
-  // counterparts that run on the UI thread.
-  static void OnPageAlmostIdleOnUi(const base::WeakPtr<TabManager>& tab_manager,
-                                   const WebContentsProxy& contents_proxy,
-                                   int64_t navigation_id);
+  // Posted to the UI thread from the GraphObserver functions above.
+  static void OnPageStoppedLoadingOnUi(const WebContentsProxy& contents_proxy);
   static void OnExpectedTaskQueueingDurationSampleOnUi(
       const base::WeakPtr<TabManager>& tab_manager,
       const WebContentsProxy& contents_proxy,

chrome/browser/ui/webui/discards/graph_dump_impl.h

@@ -120,8 +120,6 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
   void OnMainFrameUrlChanged(
       const performance_manager::PageNode* page_node) override;
   // Ignored.
-  void OnPageAlmostIdleChanged(
-      const performance_manager::PageNode* page_node) override {}
   void OnMainFrameDocumentChanged(
       const performance_manager::PageNode* page_node) override {}
   void OnHadFormInteractionChanged(

chrome/test/BUILD.gn

@@ -1071,6 +1071,7 @@ if (!is_android) {
       "../browser/pdf/pdf_extension_test_util.cc",
       "../browser/pdf/pdf_extension_test_util.h",
       "../browser/performance_manager/graph/page_node_impl_browsertest.cc",
+      "../browser/performance_manager/page_load_tracker_decorator_browsertest.cc",
       "../browser/permissions/permission_delegation_browsertest.cc",
       "../browser/permissions/permission_manager_browsertest.cc",
       "../browser/permissions/permission_request_manager_browsertest.cc",

components/performance_manager/BUILD.gn

@@ -4,9 +4,10 @@
 
 static_library("performance_manager") {
   sources = [
-    "decorators/page_almost_idle_decorator.cc",
-    "decorators/page_almost_idle_decorator.h",
     "decorators/page_live_state_decorator.cc",
+    "decorators/page_load_tracker_decorator.cc",
+    "decorators/page_load_tracker_decorator.h",
+    "decorators/page_load_tracker_decorator_helper.cc",
     "embedder/performance_manager_lifetime.h",
     "embedder/performance_manager_registry.h",
     "frame_priority/boosting_vote_aggregator.cc",
@@ -58,6 +59,7 @@ static_library("performance_manager") {
     "process_node_source.cc",
     "process_node_source.h",
     "public/decorators/page_live_state_decorator.h",
+    "public/decorators/page_load_tracker_decorator_helper.h",
     "public/frame_priority/boosting_vote_aggregator.h",
     "public/frame_priority/frame_priority.h",
     "public/frame_priority/max_vote_aggregator.h",
@@ -100,8 +102,8 @@ source_set("unit_tests") {
   testonly = true
 
   sources = [
-    "decorators/page_almost_idle_decorator_unittest.cc",
     "decorators/page_live_state_decorator_unittest.cc",
+    "decorators/page_load_tracker_decorator_unittest.cc",
     "frame_priority/boosting_vote_aggregator_unittest.cc",
     "frame_priority/frame_priority_unittest.cc",
     "frame_priority/max_vote_aggregator_unittest.cc",

components/performance_manager/decorators/page_almost_idle_decorator.h → components/performance_manager/decorators/page_load_tracker_decorator.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
+// Copyright 2020 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_PERFORMANCE_MANAGER_DECORATORS_PAGE_ALMOST_IDLE_DECORATOR_H_
-#define COMPONENTS_PERFORMANCE_MANAGER_DECORATORS_PAGE_ALMOST_IDLE_DECORATOR_H_
+#ifndef COMPONENTS_PERFORMANCE_MANAGER_DECORATORS_PAGE_LOAD_TRACKER_DECORATOR_H_
+#define COMPONENTS_PERFORMANCE_MANAGER_DECORATORS_PAGE_LOAD_TRACKER_DECORATOR_H_
 
 #include "base/time/time.h"
 #include "base/timer/timer.h"
@@ -18,19 +18,19 @@ class FrameNodeImpl;
 class PageNodeImpl;
 class ProcessNodeImpl;
 
-// The PageAlmostIdle decorator is responsible for determining when a page has
-// reached an "almost idle" state after initial load, based on CPU and network
-// quiescence, as well as an absolute timeout. This state is then updated on
-// PageNodes in a graph.
-class PageAlmostIdleDecorator : public FrameNode::ObserverDefaultImpl,
-                                public GraphOwnedDefaultImpl,
-                                public PageNode::ObserverDefaultImpl,
-                                public ProcessNode::ObserverDefaultImpl {
+// The PageLoadTracker decorator is responsible for determining when a page is
+// loading. A page starts loading when incoming data starts arriving for a
+// top-level load to a different document. It stops loading when it reaches an
+// "almost idle" state, based on CPU and network quiescence, or after an
+// absolute timeout. This state is then updated on PageNodes in a graph.
+class PageLoadTrackerDecorator : public FrameNode::ObserverDefaultImpl,
+                                 public GraphOwnedDefaultImpl,
+                                 public ProcessNode::ObserverDefaultImpl {
  public:
   class Data;
 
-  PageAlmostIdleDecorator();
-  ~PageAlmostIdleDecorator() override;
+  PageLoadTrackerDecorator();
+  ~PageLoadTrackerDecorator() override;
 
   // FrameNodeObserver implementation:
   void OnNetworkAlmostIdleChanged(const FrameNode* frame_node) override;
@@ -39,15 +39,17 @@ class PageAlmostIdleDecorator : public FrameNode::ObserverDefaultImpl,
   void OnPassedToGraph(Graph* graph) override;
   void OnTakenFromGraph(Graph* graph) override;
 
-  // PageNodeObserver implementation:
-  void OnIsLoadingChanged(const PageNode* page_node) override;
-  void OnMainFrameDocumentChanged(const PageNode* page_node) override;
-
   // ProcessNodeObserver implementation:
   void OnMainThreadTaskLoadIsLow(const ProcessNode* process_node) override;
 
+  // Invoked by PageLoadTrackerDecoratorHelper when corresponding
+  // WebContentsObserver methods are invoked, and the WebContents is loading to
+  // a different document.
+  static void DidReceiveResponse(PageNodeImpl* page_node);
+  static void DidStopLoading(PageNodeImpl* page_node);
+
  protected:
-  friend class PageAlmostIdleDecoratorTest;
+  friend class PageLoadTrackerDecoratorTest;
 
   // The amount of time a page has to be idle post-loading in order for it to be
   // considered loaded and idle. This is used in UpdateLoadIdleState
@@ -78,27 +80,28 @@ class PageAlmostIdleDecorator : public FrameNode::ObserverDefaultImpl,
   // observed. Frame and Process variants will eventually all redirect to the
   // appropriate Page variant, where the real work is done.
   void UpdateLoadIdleStateFrame(FrameNodeImpl* frame_node);
-  void UpdateLoadIdleStatePage(PageNodeImpl* page_node);
   void UpdateLoadIdleStateProcess(ProcessNodeImpl* process_node);
+  static void UpdateLoadIdleStatePage(PageNodeImpl* page_node);
 
   // Helper function for transitioning to the final state.
-  void TransitionToLoadedAndIdle(PageNodeImpl* page_node);
+  static void TransitionToLoadedAndIdle(PageNodeImpl* page_node);
 
   static bool IsIdling(const PageNodeImpl* page_node);
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(PageAlmostIdleDecorator);
+  DISALLOW_COPY_AND_ASSIGN(PageLoadTrackerDecorator);
 };
 
-class PageAlmostIdleDecorator::Data {
+class PageLoadTrackerDecorator::Data {
  public:
-  // The state transitions for the PageAlmostIdle signal. In general a page
-  // transitions through these states from top to bottom.
+  // The state transitions associated with a load. In general a page transitions
+  // through these states from top to bottom.
   enum class LoadIdleState {
     // The initial state. Can only transition to kLoading from here.
     kLoadingNotStarted,
-    // Loading has started. Almost idle signals are ignored in this state.
-    // Can transition to kLoadedNotIdling and kLoadedAndIdling from here.
+    // Incoming data has started to arrive for a load. Almost idle signals are
+    // ignored in this state. Can transition to kLoadedNotIdling and
+    // kLoadedAndIdling from here.
     kLoading,
     // Loading has completed, but the page has not started idling. Can only
     // transition to kLoadedAndIdling from here.
@@ -117,6 +120,17 @@ class PageAlmostIdleDecorator::Data {
   static Data* GetForTesting(PageNodeImpl* page_node);
   static bool DestroyForTesting(PageNodeImpl* page_node);
 
+  // Sets the LoadIdleState for the page, and updates PageNode::IsLoading()
+  // accordingly.
+  void SetLoadIdleState(PageNodeImpl* page_node, LoadIdleState load_idle_state);
+
+  // Returns the LoadIdleState for the page.
+  LoadIdleState load_idle_state() const { return load_idle_state_; }
+
+  // Whether there is an ongoing different-document load for which data started
+  // arriving.
+  bool loading_received_response_ = false;
+
   // Marks the point in time when the DidStopLoading signal was received,
   // transitioning to kLoadedAndNotIdling or kLoadedAndIdling. This is used as
   // the basis for the kWaitingForIdleTimeout.
@@ -130,6 +144,7 @@ class PageAlmostIdleDecorator::Data {
   // kLoadedAndIdle.
   base::OneShotTimer idling_timer_;
 
+ private:
   // Initially at kLoadingNotStarted. Transitions through the states via calls
   // to UpdateLoadIdleState. Is reset to kLoadingNotStarted when a non-same
   // document navigation is committed.
@@ -138,4 +153,4 @@ class PageAlmostIdleDecorator::Data {
 
 }  // namespace performance_manager
 
-#endif  // COMPONENTS_BROWSER_PERFORMANCE_MANAGER_DECORATORS_PAGE_ALMOST_IDLE_DECORATOR_H_
+#endif  // COMPONENTS_BROWSER_PERFORMANCE_MANAGER_DECORATORS_PAGE_LOAD_TRACKER_DECORATOR_H_

components/performance_manager/decorators/page_load_tracker_decorator_helper.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/performance_manager/public/decorators/page_load_tracker_decorator_helper.h"
+
+#include "base/bind.h"
+#include "components/performance_manager/decorators/page_load_tracker_decorator.h"
+#include "components/performance_manager/graph/page_node_impl.h"
+#include "components/performance_manager/performance_manager_impl.h"
+#include "components/performance_manager/public/performance_manager.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+
+namespace performance_manager {
+
+namespace {
+
+void NotifyPageLoadTrackerDecoratorOnPMSequence(content::WebContents* contents,
+                                                void (*method)(PageNodeImpl*)) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  PerformanceManagerImpl::CallOnGraphImpl(
+      FROM_HERE,
+      base::BindOnce(
+          [](base::WeakPtr<PageNode> node, void (*method)(PageNodeImpl*),
+             GraphImpl* graph) {
+            if (node) {
+              PageNodeImpl* page_node = PageNodeImpl::FromNode(node.get());
+              method(page_node);
+            }
+          },
+          PerformanceManager::GetPageNodeForWebContents(contents), method));
+}
+
+}  // namespace
+
+// Listens to content::WebContentsObserver notifications for a given WebContents
+// and updates the PageLoadTracker accordingly. Destroys itself when the
+// WebContents it observes is destroyed.
+class PageLoadTrackerDecoratorHelper::WebContentsObserver
+    : public content::WebContentsObserver {
+ public:
+  explicit WebContentsObserver(content::WebContents* web_contents,
+                               PageLoadTrackerDecoratorHelper* outer)
+      : content::WebContentsObserver(web_contents),
+        outer_(outer),
+        prev_(nullptr),
+        next_(outer->first_web_contents_observer_) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    DCHECK(web_contents);
+
+    if (next_) {
+      DCHECK(!next_->prev_);
+      next_->prev_ = this;
+    }
+    outer_->first_web_contents_observer_ = this;
+
+    if (web_contents->IsLoadingToDifferentDocument() &&
+        !web_contents->IsWaitingForResponse()) {
+      // Simulate receiving the missed DidReceiveResponse() notification.
+      DidReceiveResponse();
+    }
+  }
+
+  WebContentsObserver(const WebContentsObserver&) = delete;
+  WebContentsObserver& operator=(const WebContentsObserver&) = delete;
+
+  ~WebContentsObserver() override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  }
+
+  // content::WebContentsObserver:
+  void DidReceiveResponse() override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+    // Only observe top-level navigation to a different document.
+    if (!web_contents()->IsLoadingToDifferentDocument())
+      return;
+
+    DCHECK(web_contents()->IsLoading());
+
+#if DCHECK_IS_ON()
+    DCHECK(!did_receive_response_);
+    did_receive_response_ = true;
+#endif
+    NotifyPageLoadTrackerDecoratorOnPMSequence(
+        web_contents(), &PageLoadTrackerDecorator::DidReceiveResponse);
+  }
+
+  void DidStopLoading() override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+#if DCHECK_IS_ON()
+    did_receive_response_ = false;
+#endif
+    NotifyPageLoadTrackerDecoratorOnPMSequence(
+        web_contents(), &PageLoadTrackerDecorator::DidStopLoading);
+  }
+
+  void WebContentsDestroyed() override {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    DetachAndDestroy();
+  }
+
+  // Removes the WebContentsObserver from the linked list and deletes it.
+  void DetachAndDestroy() {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+    if (prev_) {
+      DCHECK_EQ(prev_->next_, this);
+      prev_->next_ = next_;
+    } else {
+      DCHECK_EQ(outer_->first_web_contents_observer_, this);
+      outer_->first_web_contents_observer_ = next_;
+    }
+    if (next_) {
+      DCHECK_EQ(next_->prev_, this);
+      next_->prev_ = prev_;
+    }
+
+    delete this;
+  }
+
+ private:
+  // TODO(https://crbug.com/1048719): Extract the logic to manage a linked list
+  // of WebContentsObservers to a helper class.
+  PageLoadTrackerDecoratorHelper* const outer_;
+  WebContentsObserver* prev_;
+  WebContentsObserver* next_;
+
+#if DCHECK_IS_ON()
+  // Used to verify the invariant that DidReceiveResponse() cannot be called
+  // twice in a row without a DidStopLoading() in between.
+  bool did_receive_response_ = false;
+#endif
+
+  SEQUENCE_CHECKER(sequence_checker_);
+};
+
+PageLoadTrackerDecoratorHelper::PageLoadTrackerDecoratorHelper() {
+  PerformanceManager::AddObserver(this);
+}
+
+PageLoadTrackerDecoratorHelper::~PageLoadTrackerDecoratorHelper() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  // Destroy all WebContentsObserver to ensure that PageLiveStateDecorators are
+  // no longer maintained.
+  while (first_web_contents_observer_)
+    first_web_contents_observer_->DetachAndDestroy();
+
+  PerformanceManager::RemoveObserver(this);
+}
+
+void PageLoadTrackerDecoratorHelper::OnPageNodeCreatedForWebContents(
+    content::WebContents* web_contents) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(web_contents);
+  // Start observing the WebContents. See comment on
+  // |first_web_contents_observer_| for lifetime management details.
+  new WebContentsObserver(web_contents, this);
+}
+
+}  // namespace performance_manager

components/performance_manager/decorators/page_almost_idle_decorator_unittest.cc → components/performance_manager/decorators/page_load_tracker_decorator_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/performance_manager/decorators/page_almost_idle_decorator.h"
+#include "components/performance_manager/decorators/page_load_tracker_decorator.h"
 
 #include <memory>
 #include <type_traits>
@@ -19,39 +19,37 @@
 
 namespace performance_manager {
 
-class PageAlmostIdleDecoratorTest : public GraphTestHarness {
+// Aliasing these here makes this unittest much more legible.
+using Data = PageLoadTrackerDecorator::Data;
+using LIS = Data::LoadIdleState;
+
+class PageLoadTrackerDecoratorTest : public GraphTestHarness {
  protected:
-  PageAlmostIdleDecoratorTest() = default;
-  ~PageAlmostIdleDecoratorTest() override = default;
+  PageLoadTrackerDecoratorTest() = default;
+  ~PageLoadTrackerDecoratorTest() override = default;
 
   void SetUp() override {
-    paid_ = new PageAlmostIdleDecorator();
-    graph()->PassToGraph(base::WrapUnique(paid_));
+    pltd_ = new PageLoadTrackerDecorator();
+    graph()->PassToGraph(base::WrapUnique(pltd_));
   }
 
   void TestPageAlmostIdleTransitions(bool timeout);
 
   bool IsIdling(const PageNodeImpl* page_node) const {
-    return PageAlmostIdleDecorator::IsIdling(page_node);
+    return PageLoadTrackerDecorator::IsIdling(page_node);
   }
 
-  PageAlmostIdleDecorator* paid_ = nullptr;
+  PageLoadTrackerDecorator* pltd_ = nullptr;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(PageAlmostIdleDecoratorTest);
+  DISALLOW_COPY_AND_ASSIGN(PageLoadTrackerDecoratorTest);
 };
 
-void PageAlmostIdleDecoratorTest::TestPageAlmostIdleTransitions(bool timeout) {
+void PageLoadTrackerDecoratorTest::TestPageAlmostIdleTransitions(bool timeout) {
   static const base::TimeDelta kLoadedAndIdlingTimeout =
-      PageAlmostIdleDecorator::kLoadedAndIdlingTimeout;
+      PageLoadTrackerDecorator::kLoadedAndIdlingTimeout;
   static const base::TimeDelta kWaitingForIdleTimeout =
-      PageAlmostIdleDecorator::kWaitingForIdleTimeout;
-
-  // Aliasing these here makes this unittest much more legible.
-  using Data = PageAlmostIdleDecorator::Data;
-  using LIS = Data::LoadIdleState;
-
-  AdvanceClock(base::TimeDelta::FromSeconds(1));
+      PageLoadTrackerDecorator::kWaitingForIdleTimeout;
 
   MockSinglePageInSingleProcessGraph mock_graph(graph());
   auto* frame_node = mock_graph.frame.get();
@@ -60,107 +58,112 @@ void PageAlmostIdleDecoratorTest::TestPageAlmostIdleTransitions(bool timeout) {
   auto* page_data = Data::GetOrCreateForTesting(page_node);
 
   // Initially the page should be in a loading not started state.
-  EXPECT_EQ(LIS::kLoadingNotStarted, page_data->load_idle_state_);
-  EXPECT_FALSE(page_data->idling_timer_.IsRunning());
-
-  // The state should not transition when a not loading state is explicitly
-  // set.
-  page_node->SetIsLoading(false);
-  EXPECT_EQ(LIS::kLoadingNotStarted, page_data->load_idle_state_);
+  EXPECT_EQ(LIS::kLoadingNotStarted, page_data->load_idle_state());
+  EXPECT_FALSE(page_node->is_loading());
   EXPECT_FALSE(page_data->idling_timer_.IsRunning());
 
-  // The state should transition to loading when loading starts.
-  page_node->SetIsLoading(true);
-  EXPECT_EQ(LIS::kLoading, page_data->load_idle_state_);
+  // The state should transition to loading when DidReceiveResponse() is called
+  // to indicate that loading starts.
+  PageLoadTrackerDecorator::DidReceiveResponse(page_node);
+  EXPECT_EQ(LIS::kLoading, page_data->load_idle_state());
+  EXPECT_TRUE(page_node->is_loading());
   EXPECT_FALSE(page_data->idling_timer_.IsRunning());
 
   // Mark the page as idling. It should transition from kLoading directly
   // to kLoadedAndIdling after this.
   frame_node->SetNetworkAlmostIdle();
   proc_node->SetMainThreadTaskLoadIsLow(true);
-  page_node->SetIsLoading(false);
-  EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state_);
+  PageLoadTrackerDecorator::DidStopLoading(page_node);
+  EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state());
+  EXPECT_TRUE(page_node->is_loading());
   EXPECT_TRUE(page_data->idling_timer_.IsRunning());
 
-  // Indicate loading is happening again. This should be ignored.
-  page_node->SetIsLoading(true);
-  EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state_);
+  // Indicate that a response was received again. This should be ignored.
+  PageLoadTrackerDecorator::DidReceiveResponse(page_node);
+  EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state());
+  EXPECT_TRUE(page_node->is_loading());
   EXPECT_TRUE(page_data->idling_timer_.IsRunning());
-  page_node->SetIsLoading(false);
-  EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state_);
+  PageLoadTrackerDecorator::DidStopLoading(page_node);
+  EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state());
+  EXPECT_TRUE(page_node->is_loading());
   EXPECT_TRUE(page_data->idling_timer_.IsRunning());
 
   // Go back to not idling. We should transition back to kLoadedNotIdling, and
   // a timer should still be running.
   frame_node->OnNavigationCommitted(GURL(), false);
-  EXPECT_EQ(LIS::kLoadedNotIdling, page_data->load_idle_state_);
+  EXPECT_FALSE(frame_node->network_almost_idle());
+  EXPECT_EQ(LIS::kLoadedNotIdling, page_data->load_idle_state());
   EXPECT_TRUE(page_data->idling_timer_.IsRunning());
 
-  base::TimeTicks start = base::TimeTicks::Now();
   if (timeout) {
     // Let the timeout run down. The final state transition should occur.
-    task_env().FastForwardUntilNoTasksRemain();
-    base::TimeTicks end = base::TimeTicks::Now();
-    base::TimeDelta elapsed = end - start;
-    EXPECT_LE(kLoadedAndIdlingTimeout, elapsed);
-    EXPECT_LE(kWaitingForIdleTimeout, elapsed);
+    task_env().FastForwardBy(kWaitingForIdleTimeout);
     EXPECT_FALSE(Data::GetForTesting(page_node));
+    EXPECT_FALSE(page_node->is_loading());
   } else {
     // Go back to idling.
     frame_node->SetNetworkAlmostIdle();
-    EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state_);
+    EXPECT_TRUE(frame_node->network_almost_idle());
+    EXPECT_EQ(LIS::kLoadedAndIdling, page_data->load_idle_state());
+    EXPECT_TRUE(page_node->is_loading());
     EXPECT_TRUE(page_data->idling_timer_.IsRunning());
 
     // Let the idle timer evaluate. The final state transition should occur.
-    task_env().FastForwardUntilNoTasksRemain();
-    base::TimeTicks end = base::TimeTicks::Now();
-    base::TimeDelta elapsed = end - start;
-    EXPECT_LE(kLoadedAndIdlingTimeout, elapsed);
-    EXPECT_GT(kWaitingForIdleTimeout, elapsed);
+    task_env().FastForwardBy(kLoadedAndIdlingTimeout);
     EXPECT_FALSE(Data::GetForTesting(page_node));
+    EXPECT_FALSE(page_node->is_loading());
   }
 
   // Firing other signals should not change the state at all.
   proc_node->SetMainThreadTaskLoadIsLow(false);
   EXPECT_FALSE(Data::GetForTesting(page_node));
+  EXPECT_FALSE(page_node->is_loading());
   frame_node->OnNavigationCommitted(GURL(), false);
+  EXPECT_FALSE(frame_node->network_almost_idle());
   EXPECT_FALSE(Data::GetForTesting(page_node));
-
-  // Post a navigation. The state should reset.
-  page_node->OnMainFrameNavigationCommitted(false, base::TimeTicks::Now(), 1,
-                                            GURL("https://www.example.org"),
-                                            "text/html");
-  page_data = Data::GetForTesting(page_node);
-  EXPECT_EQ(LIS::kLoadingNotStarted, page_data->load_idle_state_);
-  EXPECT_FALSE(page_data->idling_timer_.IsRunning());
+  EXPECT_FALSE(page_node->is_loading());
 }
 
-TEST_F(PageAlmostIdleDecoratorTest, TestTransitionsNoTimeout) {
+TEST_F(PageLoadTrackerDecoratorTest, TestTransitionsNoTimeout) {
   TestPageAlmostIdleTransitions(false);
 }
 
-TEST_F(PageAlmostIdleDecoratorTest, TestTransitionsWithTimeout) {
+TEST_F(PageLoadTrackerDecoratorTest, TestTransitionsWithTimeout) {
   TestPageAlmostIdleTransitions(true);
 }
 
-TEST_F(PageAlmostIdleDecoratorTest, IsLoading) {
+TEST_F(PageLoadTrackerDecoratorTest, TestTransitionsNotIdlingOnDidStopLoading) {
   MockSinglePageInSingleProcessGraph mock_graph(graph());
+  auto* frame_node = mock_graph.frame.get();
   auto* page_node = mock_graph.page.get();
+  auto* proc_node = mock_graph.process.get();
+  auto* page_data = Data::GetOrCreateForTesting(page_node);
 
-  // The loading property hasn't yet been set. Then IsLoading should return
-  // false as the default value.
+  // Initially the page should be in a loading not started state.
+  EXPECT_EQ(LIS::kLoadingNotStarted, page_data->load_idle_state());
   EXPECT_FALSE(page_node->is_loading());
+  EXPECT_FALSE(page_data->idling_timer_.IsRunning());
 
-  // Once the loading property has been set it should return that value.
-  page_node->SetIsLoading(false);
-  EXPECT_FALSE(page_node->is_loading());
-  page_node->SetIsLoading(true);
+  // The state should transition to loading when DidReceiveResponse() is called
+  // to indicate that loading starts.
+  PageLoadTrackerDecorator::DidReceiveResponse(page_node);
+  EXPECT_EQ(LIS::kLoading, page_data->load_idle_state());
   EXPECT_TRUE(page_node->is_loading());
-  page_node->SetIsLoading(false);
-  EXPECT_FALSE(page_node->is_loading());
+  EXPECT_FALSE(page_data->idling_timer_.IsRunning());
+
+  // Mark the page as not idling.
+  frame_node->OnNavigationCommitted(GURL(), false);
+  proc_node->SetMainThreadTaskLoadIsLow(false);
+  EXPECT_FALSE(IsIdling(page_node));
+
+  // DidStopLoading() should cause a transition to kLoadedNotIdling.
+  PageLoadTrackerDecorator::DidStopLoading(page_node);
+  EXPECT_EQ(LIS::kLoadedNotIdling, page_data->load_idle_state());
+  EXPECT_TRUE(page_node->is_loading());
+  EXPECT_TRUE(page_data->idling_timer_.IsRunning());
 }
 
-TEST_F(PageAlmostIdleDecoratorTest, IsIdling) {
+TEST_F(PageLoadTrackerDecoratorTest, IsIdling) {
   MockSinglePageInSingleProcessGraph mock_graph(graph());
   auto* frame_node = mock_graph.frame.get();
   auto* page_node = mock_graph.page.get();

components/performance_manager/graph/page_node_impl.cc

@@ -232,11 +232,6 @@ const std::string& PageNodeImpl::browser_context_id() const {
   return browser_context_id_;
 }
 
-bool PageNodeImpl::page_almost_idle() const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return page_almost_idle_.value();
-}
-
 const GURL& PageNodeImpl::main_frame_url() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return main_frame_url_.value();
@@ -306,11 +301,6 @@ const std::string& PageNodeImpl::GetBrowserContextID() const {
   return browser_context_id();
 }
 
-bool PageNodeImpl::IsPageAlmostIdle() const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return page_almost_idle();
-}
-
 bool PageNodeImpl::IsVisible() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   return is_visible();
@@ -397,11 +387,6 @@ const WebContentsProxy& PageNodeImpl::GetContentsProxy() const {
   return contents_proxy();
 }
 
-void PageNodeImpl::SetPageAlmostIdle(bool page_almost_idle) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  page_almost_idle_.SetAndMaybeNotify(this, page_almost_idle);
-}
-
 void PageNodeImpl::SetLifecycleState(LifecycleState lifecycle_state) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   lifecycle_state_.SetAndMaybeNotify(this, lifecycle_state);

components/performance_manager/graph/page_node_impl.h

@@ -89,7 +89,6 @@ class PageNodeImpl
   base::TimeTicks usage_estimate_time() const;
   base::TimeDelta cumulative_cpu_usage_estimate() const;
   uint64_t private_footprint_kb_estimate() const;
-  bool page_almost_idle() const;
   const GURL& main_frame_url() const;
   int64_t navigation_id() const;
   const std::string& contents_mime_type() const;
@@ -106,10 +105,6 @@ class PageNodeImpl
     SetLifecycleState(lifecycle_state);
   }
 
-  void SetPageAlmostIdleForTesting(bool page_almost_idle) {
-    SetPageAlmostIdle(page_almost_idle);
-  }
-
   void SetIsHoldingWebLockForTesting(bool is_holding_weblock) {
     SetIsHoldingWebLock(is_holding_weblock);
   }
@@ -126,11 +121,10 @@ class PageNodeImpl
   friend class FrameNodeImpl;
   friend class PageAggregatorAccess;
   friend class FrozenFrameAggregatorAccess;
-  friend class PageAlmostIdleAccess;
+  friend class PageLoadTrackerAccess;
 
   // PageNode implementation:
   const std::string& GetBrowserContextID() const override;
-  bool IsPageAlmostIdle() const override;
   bool IsVisible() const override;
   base::TimeDelta GetTimeSinceLastVisibilityChange() const override;
   bool IsAudible() const override;
@@ -154,7 +148,6 @@ class PageNodeImpl
   void JoinGraph() override;
   void LeaveGraph() override;
 
-  void SetPageAlmostIdle(bool page_almost_idle);
   void SetLifecycleState(LifecycleState lifecycle_state);
   void SetOriginTrialFreezePolicy(InterventionPolicy policy);
   void SetIsHoldingWebLock(bool is_holding_weblock);
@@ -217,11 +210,6 @@ class PageNodeImpl
   // The unique ID of the browser context that this page belongs to.
   const std::string browser_context_id_;
 
-  // Page almost idle state. This is the output that is driven by the
-  // PageAlmostIdleDecorator.
-  ObservedProperty::
-      NotifiesOnlyOnChanges<bool, &PageNodeObserver::OnPageAlmostIdleChanged>
-          page_almost_idle_{false};
   // Whether or not the page is visible. Driven by browser instrumentation.
   // Initialized on construction.
   ObservedProperty::NotifiesOnlyOnChanges<bool,
@@ -272,8 +260,8 @@ class PageNodeImpl
       &PageNodeObserver::OnHadFormInteractionChanged>
       had_form_interaction_{false};
 
-  // Storage for PageAlmostIdle user data.
-  std::unique_ptr<NodeAttachedData> page_almost_idle_data_;
+  // Storage for PageLoadTracker user data.
+  std::unique_ptr<NodeAttachedData> page_load_tracker_data_;
 
   // Inline storage for FrozenFrameAggregator user data.
   InternalNodeAttachedDataStorage<sizeof(uintptr_t) + 8> frozen_frame_data_;

components/performance_manager/graph/page_node_impl_unittest.cc

@@ -232,7 +232,6 @@ class LenientMockObserver : public PageNodeImpl::Observer {
   MOCK_METHOD1(OnPageIsHoldingWebLockChanged, void(const PageNode*));
   MOCK_METHOD1(OnPageIsHoldingIndexedDBLockChanged, void(const PageNode*));
   MOCK_METHOD1(OnMainFrameUrlChanged, void(const PageNode*));
-  MOCK_METHOD1(OnPageAlmostIdleChanged, void(const PageNode*));
   MOCK_METHOD1(OnMainFrameDocumentChanged, void(const PageNode*));
   MOCK_METHOD1(OnTitleUpdated, void(const PageNode*));
   MOCK_METHOD1(OnFaviconUpdated, void(const PageNode*));
@@ -297,11 +296,6 @@ TEST_F(PageNodeImplTest, ObserverWorks) {
   page_node->SetLifecycleStateForTesting(PageNodeImpl::LifecycleState::kFrozen);
   EXPECT_EQ(raw_page_node, obs.TakeNotifiedPageNode());
 
-  EXPECT_CALL(obs, OnPageAlmostIdleChanged(_))
-      .WillOnce(Invoke(&obs, &MockObserver::SetNotifiedPageNode));
-  page_node->SetPageAlmostIdleForTesting(true);
-  EXPECT_EQ(raw_page_node, obs.TakeNotifiedPageNode());
-
   const GURL kTestUrl = GURL("https://foo.com/");
   int64_t navigation_id = 0x1234;
   EXPECT_CALL(obs, OnMainFrameUrlChanged(_))
@@ -345,8 +339,6 @@ TEST_F(PageNodeImplTest, PublicInterface) {
 
   EXPECT_EQ(page_node->browser_context_id(),
             public_page_node->GetBrowserContextID());
-  EXPECT_EQ(page_node->page_almost_idle(),
-            public_page_node->IsPageAlmostIdle());
   EXPECT_EQ(page_node->is_visible(), public_page_node->IsVisible());
   EXPECT_EQ(page_node->is_audible(), public_page_node->IsAudible());
   EXPECT_EQ(page_node->is_loading(), public_page_node->IsLoading());

components/performance_manager/performance_manager_lifetime.cc

@@ -5,7 +5,7 @@
 #include "components/performance_manager/embedder/performance_manager_lifetime.h"
 
 #include "base/bind.h"
-#include "components/performance_manager/decorators/page_almost_idle_decorator.h"
+#include "components/performance_manager/decorators/page_load_tracker_decorator.h"
 #include "components/performance_manager/performance_manager_impl.h"
 #include "components/performance_manager/public/graph/graph.h"
 
@@ -16,7 +16,7 @@ namespace {
 void DefaultGraphCreatedCallback(
     GraphCreatedCallback external_graph_created_callback,
     GraphImpl* graph) {
-  graph->PassToGraph(std::make_unique<PageAlmostIdleDecorator>());
+  graph->PassToGraph(std::make_unique<PageLoadTrackerDecorator>());
   std::move(external_graph_created_callback).Run(graph);
 }
 

components/performance_manager/performance_manager_tab_helper.cc

@@ -213,14 +213,6 @@ void PerformanceManagerTabHelper::RenderFrameHostChanged(
                      old_frame, new_frame));
 }
 
-void PerformanceManagerTabHelper::DidStartLoading() {
-  PostToGraph(FROM_HERE, &PageNodeImpl::SetIsLoading, page_node_.get(), true);
-}
-
-void PerformanceManagerTabHelper::DidStopLoading() {
-  PostToGraph(FROM_HERE, &PageNodeImpl::SetIsLoading, page_node_.get(), false);
-}
-
 void PerformanceManagerTabHelper::OnVisibilityChanged(
     content::Visibility visibility) {
   const bool is_visible = visibility == content::Visibility::VISIBLE;

components/performance_manager/performance_manager_tab_helper.h

@@ -62,8 +62,6 @@ class PerformanceManagerTabHelper
   void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
   void RenderFrameHostChanged(content::RenderFrameHost* old_host,
                               content::RenderFrameHost* new_host) override;
-  void DidStartLoading() override;
-  void DidStopLoading() override;
   void OnVisibilityChanged(content::Visibility visibility) override;
   void OnAudioStateChanged(bool audible) override;
   void DidFinishNavigation(

components/performance_manager/public/decorators/page_load_tracker_decorator_helper.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_DECORATORS_PAGE_LOAD_TRACKER_DECORATOR_HELPER_H_
+#define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_DECORATORS_PAGE_LOAD_TRACKER_DECORATOR_HELPER_H_
+
+#include "components/performance_manager/public/performance_manager_main_thread_observer.h"
+
+namespace performance_manager {
+
+// This class must be instantiated on the UI thread in order to maintain the
+// PageLoadTracker decorator of PageNodes.
+class PageLoadTrackerDecoratorHelper
+    : public PerformanceManagerMainThreadObserver {
+ public:
+  PageLoadTrackerDecoratorHelper();
+  ~PageLoadTrackerDecoratorHelper() override;
+  PageLoadTrackerDecoratorHelper(const PageLoadTrackerDecoratorHelper& other) =
+      delete;
+  PageLoadTrackerDecoratorHelper& operator=(
+      const PageLoadTrackerDecoratorHelper&) = delete;
+
+  // PerformanceManagerMainThreadObserver:
+  void OnPageNodeCreatedForWebContents(
+      content::WebContents* web_contents) override;
+
+ private:
+  class WebContentsObserver;
+
+  // Linked list of WebContentsObservers created by this
+  // PageLoadTrackerDecoratorHelper. Each WebContentsObservers removes itself
+  // from the list and destroys itself when its associated WebContents is
+  // destroyed. Additionally, all WebContentsObservers that are still in this
+  // list when the destructor of PageLoadTrackerDecoratorHelper is invoked are
+  // destroyed.
+  WebContentsObserver* first_web_contents_observer_ = nullptr;
+};
+
+}  // namespace performance_manager
+
+#endif  // COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_DECORATORS_PAGE_LOAD_TRACKER_DECORATOR_HELPER_H_

components/performance_manager/public/graph/page_node.h

@@ -38,10 +38,6 @@ class PageNode : public Node {
   // Returns the unique ID of the browser context that this page belongs to.
   virtual const std::string& GetBrowserContextID() const = 0;
 
-  // Returns the page almost idle state of this page.
-  // See PageNodeObserver::OnPageAlmostIdleChanged.
-  virtual bool IsPageAlmostIdle() const = 0;
-
   // Returns true if this page is currently visible, false otherwise.
   // See PageNodeObserver::OnIsVisibleChanged.
   virtual bool IsVisible() const = 0;
@@ -54,8 +50,12 @@ class PageNode : public Node {
   // See PageNodeObserver::OnIsAudibleChanged.
   virtual bool IsAudible() const = 0;
 
-  // Returns true if this page is currently loading, false otherwise.
-  // See PageNodeObserver::OnIsLoadingChanged.
+  // Returns true if this page is currently loading, false otherwise. The page
+  // starts loading when incoming data starts arriving for a top-level load to a
+  // different document. It stops loading when it reaches an "almost idle"
+  // state, based on CPU and network quiescence, or after an absolute timeout.
+  // Note: This is different from WebContents::IsLoading(). See
+  // PageNodeObserver::OnIsLoadingChanged.
   virtual bool IsLoading() const = 0;
 
   // Returns the UKM source ID associated with the URL of the main frame of
@@ -166,9 +166,6 @@ class PageNodeObserver {
   // Invoked when the MainFrameUrl property changes.
   virtual void OnMainFrameUrlChanged(const PageNode* page_node) = 0;
 
-  // Invoked when the PageAlmostIdle property changes.
-  virtual void OnPageAlmostIdleChanged(const PageNode* page_node) = 0;
-
   // This is fired when a non-same document navigation commits in the main
   // frame. It indicates that the the |NavigationId| property and possibly the
   // |MainFrameUrl| properties have changed.
@@ -212,7 +209,6 @@ class PageNode::ObserverDefaultImpl : public PageNodeObserver {
   void OnPageIsHoldingWebLockChanged(const PageNode* page_node) override {}
   void OnPageIsHoldingIndexedDBLockChanged(const PageNode* page_node) override {
   }
-  void OnPageAlmostIdleChanged(const PageNode* page_node) override {}
   void OnMainFrameUrlChanged(const PageNode* page_node) override {}
   void OnMainFrameDocumentChanged(const PageNode* page_node) override {}
   void OnHadFormInteractionChanged(const PageNode* page_node) override {}