Wait for correct event in test

The test ChildFrameCrashMetrics_ScrolledIntoView might fail if we wait
for WidgetHostMsg_WaitForNextFrameForTests_ACK, because it is sent
from compositor thread, almost independently from
FrameHostMsg_UpdateViewportIntersection, which is sent from main thread.

Problem can be reproduced deterministically if we put sleep in
RenderFrameProxy::UpdateRemoteViewportIntersection, to simulate preemption
by OS scheduler.

What we really want to wait for in this test is viewport intersection
update in UI thread. Only after that we can check histograms. If
we wait for lifecycle update, that will guarantee that all IPCs are
processed in browser, including FrameHostMsg_UpdateViewportIntersection.

Change-Id: I1ff2e6632b2f1391a3dee82b2bc6667bb20bf71d
Bug: 1115096
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307373
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814114}

content/browser/site_per_process_browsertest.cc

@@ -13585,9 +13585,11 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
     }
   )";
   EXPECT_TRUE(ExecuteScript(root, filling_script));
-  MainThreadFrameObserver main_widget_observer(
-      root->current_frame_host()->GetRenderWidgetHost());
-  main_widget_observer.Wait();
+  // This will ensure that browser has received the
+  // FrameHostMsg_UpdateViewportIntersection IPC message from the renderer main
+  // thread.
+  EXPECT_EQ(true,
+            EvalJsAfterLifecycleUpdate(root->current_frame_host(), "", "true"));
 
   // Kill the child frame.
   base::HistogramTester histograms;
@@ -13611,7 +13613,9 @@ IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTest,
     frame.scrollIntoView();
   )";
   EXPECT_TRUE(ExecuteScript(root, scrolling_script));
-  main_widget_observer.Wait();
+  // Wait for FrameHostMsg_UpdateViewportIntersection again.
+  EXPECT_EQ(true,
+            EvalJsAfterLifecycleUpdate(root->current_frame_host(), "", "true"));
 
   // Verify that the expected metrics got logged.
   histograms.ExpectUniqueSample(
@@ -13638,10 +13642,8 @@ class SitePerProcessBrowserTestWithoutSadFrameTabReload
   base::test::ScopedFeatureList feature_list_;
 };
 
-// Flaky everywhere: https://crbug.com/1115096
-IN_PROC_BROWSER_TEST_P(
-    SitePerProcessBrowserTestWithoutSadFrameTabReload,
-    DISABLED_ChildFrameCrashMetrics_ScrolledIntoViewAfterTabIsShown) {
+IN_PROC_BROWSER_TEST_P(SitePerProcessBrowserTestWithoutSadFrameTabReload,
+                       ChildFrameCrashMetrics_ScrolledIntoViewAfterTabIsShown) {
   // Start on a page that has a single iframe, which is positioned out of
   // view, and navigate that iframe cross-site.
   GURL main_url(
@@ -13679,14 +13681,16 @@ IN_PROC_BROWSER_TEST_P(
 
   // Scroll the subframe into view and wait until the scrolled frame draws
   // itself.
-  MainThreadFrameObserver main_widget_observer(
-      root->current_frame_host()->GetRenderWidgetHost());
   std::string scrolling_script = R"(
     var frame = document.body.querySelector("iframe");
     frame.scrollIntoView();
   )";
   EXPECT_TRUE(ExecuteScript(root, scrolling_script));
-  main_widget_observer.Wait();
+  // This will ensure that browser has received the
+  // FrameHostMsg_UpdateViewportIntersection IPC message from the renderer main
+  // thread.
+  EXPECT_EQ(true,
+            EvalJsAfterLifecycleUpdate(root->current_frame_host(), "", "true"));
 
   // Verify that the expected metrics got logged.
   histograms.ExpectUniqueSample(

content/public/test/browser_test_utils.h

@@ -1321,12 +1321,10 @@ class RenderFrameSubmissionObserver
 // This is accomplished by sending an IPC to RenderWidget, then blocking until
 // the ACK is received and processed.
 //
-// When the main thread receives the ACK it is enqueued. The queue is not
-// processed until a new FrameToken is received.
-//
-// So while the ACK can arrive before a CompositorFrame submission occurs. The
-// processing does not occur until after the FrameToken for that frame
-// submission arrives to the main thread.
+// The ACK is sent from compositor thread, when the CompositorFrame is submited
+// to the display compositor
+// TODO(danakj): This class seems to provide the same as
+// RenderFrameSubmissionObserver, consider using that instead.
 class MainThreadFrameObserver {
  public:
   explicit MainThreadFrameObserver(RenderWidgetHost* render_widget_host);