Partial reland 2/2: [metrics] Start loading visible tabs before creating FirstWebContentsProfiler.

This is a partial reland of https://crrev.com/c/chromium/src/+/2062692.
We confirmed that the CL affected startup metrics on Mac. However, our
assessment is that the change is correct (restored tabs are painted
before non-restored tabs more often). As a precaution, the
original CL is split in two:
- Start loading restored tabs earlier (https://crrev.com/c/chromium/src/+/2095970)
- Enforce stricter requirements in FirstWebContentsProfiler (this CL)

Original change description:

On Windows, a visible restored tab starts loading in the following stack,
before FirstWebContentsProfiler is created:

  content::NavigationControllerImpl::LoadIfNecessary
  content::NavigationControllerImpl::SetActive
  ...
  content::WebContentsImpl::UpdateWebContentsVisibility
  content::WebContentsViewAura::UpdateWebContentsVisibility
  ...
  BrowserView::Show
  SessionRestoreImpl::ShowBrowser
  SessionRestoreImpl::RestoreTab
  SessionRestoreImpl::RestoreTabsToBrowser
  SessionRestoreImpl::ProcessSessionWindows
  SessionRestoreImpl::ProcessSessionWindowsAndNotify
  SessionRestoreImpl::Restore
  SessionRestore::RestoreSession
  ...
  StartupBrowserCreator::Start
  ...
  content::BrowserMainLoop::PreMainMessageLoopRun

On Mac, it starts loading in the following stack, after
FirstWebContentsProfiler is created:

  content::NavigationControllerImpl::LoadIfNecessary
  content::NavigationControllerImpl::SetActive
  ...
  content::WebContentsImpl::UpdateWebContentsVisibility
  content::WebContentsViewMac::OnWindowVisibilityChanged
  -[WebContentsViewCocoa updateWebContentsVisibility]
  -[WebContentsViewCocoa windowChangedOcclusionState:]
  ...
  base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run
  ...
  ChromeBrowserMainParts::MainMessageLoopRun

This prevents FirstWebContentsProfiler from having strict cross-platform
expectations about events it observes. With this CL, we ensure that visible
restored tab start loading in the following stack on all platforms:

  content::NavigationControllerImpl::LoadIfNecessary
  chrome::`anonymous namespace'::CreateRestoredTab
  chrome::AddRestoredTab
  ...
  SessionRestoreImpl::Restore
  SessionRestore::RestoreSession
  ...
  StartupBrowserCreator::Start
  ...
  content::BrowserMainLoop::PreMainMessageLoopRun

This allows us to enforce strict cross-platform expectations about events
observed by FirstWebContentsProfiler, and also have the nice benefits that
navigation isn't delayed until we get a window visibility update during a
Mac session restore.

Bug: 1035419, 1020549, 1022492
Change-Id: I6885f0fc667423bc55cdc201c9ada4dca167ce92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097368Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752059}

chrome/browser/metrics/first_web_contents_profiler.cc

@@ -55,10 +55,6 @@ enum class FinishReason {
   kMaxValue = kAbandonNoInitiallyVisibleContent
 };
 
-// Per documentation in navigation_request.cc, a navigation id is guaranteed
-// nonzero.
-constexpr int64_t kInvalidNavigationId = 0;
-
 void RecordFinishReason(FinishReason finish_reason) {
   base::UmaHistogramEnumeration("Startup.FirstWebContents.FinishReason",
                                 finish_reason);
@@ -85,9 +81,6 @@ class FirstWebContentsProfiler : public content::WebContentsObserver {
   // Logs |finish_reason| to UMA and deletes this FirstWebContentsProfiler.
   void FinishedCollectingMetrics(FinishReason finish_reason);
 
-  // The first NavigationHandle id observed by this.
-  int64_t first_navigation_id_ = kInvalidNavigationId;
-
   // Whether a main frame navigation finished since this was created.
   bool did_finish_first_navigation_ = false;
 
@@ -103,6 +96,11 @@ FirstWebContentsProfiler::FirstWebContentsProfiler(
     : content::WebContentsObserver(web_contents),
       memory_pressure_listener_(base::BindRepeating(
           &startup_metric_utils::OnMemoryPressureBeforeFirstNonEmptyPaint)) {
+  // FirstWebContentsProfiler is created before the main MessageLoop starts
+  // running. At that time, any visible WebContents should have a pending
+  // NavigationEntry, i.e. should have dispatched DidStartNavigation() but not
+  // DidFinishNavigation().
+  DCHECK(web_contents->GetController().GetPendingEntry());
 }
 
 void FirstWebContentsProfiler::DidStartNavigation(
@@ -113,22 +111,11 @@ void FirstWebContentsProfiler::DidStartNavigation(
     return;
   }
 
-  // TODO(https://crbug.com/1035419): Ensure that all visible tabs start
-  // navigating before FirstWebContentsProfiler creation and always handle
-  // a top-level DidStartNavigation() as a new navigation.
-  // Upcoming CL: https://crrev.com/c/chromium/src/+/1976500
-
-  if (first_navigation_id_ != kInvalidNavigationId) {
-    // Abandon if this is not the first observed top-level navigation.
-    DCHECK_NE(first_navigation_id_, navigation_handle->GetNavigationId());
-    FinishedCollectingMetrics(FinishReason::kAbandonNewNavigation);
-    return;
-  }
-
-  DCHECK(!did_finish_first_navigation_);
-
-  // Keep track of the first top-level navigation id observed by this.
-  first_navigation_id_ = navigation_handle->GetNavigationId();
+  // FirstWebContentsProfiler is created after DidStartNavigation() has been
+  // dispatched for the first top-level navigation. If another
+  // DidStartNavigation() is received, it means that a new navigation was
+  // initiated.
+  FinishedCollectingMetrics(FinishReason::kAbandonNewNavigation);
 }
 
 void FirstWebContentsProfiler::DidFinishNavigation(
@@ -150,21 +137,10 @@ void FirstWebContentsProfiler::DidFinishNavigation(
     return;
   }
 
-  if (first_navigation_id_ == kInvalidNavigationId) {
-    // Keep track of the first top-level navigation id observed by this.
-    //
-    // Note: FirstWebContentsProfiler may be created before or after
-    // DidStartNavigation() is dispatched for the first navigation, which is why
-    // |first_navigation_id_| may be set in DidStartNavigation() or
-    // DidFinishNavigation().
-    first_navigation_id_ = navigation_handle->GetNavigationId();
-  } else if (first_navigation_id_ != navigation_handle->GetNavigationId()) {
-    // Abandon if this is not the first observed top-level navigation.
-    FinishedCollectingMetrics(FinishReason::kAbandonNewNavigation);
-    return;
-  }
-
+  // It is not possible to get a second top-level DidFinishNavigation() without
+  // first having a DidStartNavigation(), which would have deleted |this|.
   DCHECK(!did_finish_first_navigation_);
+
   did_finish_first_navigation_ = true;
 
   startup_metric_utils::RecordFirstWebContentsMainNavigationStart(