content/viz: Fix and re-enable some tests

Some tests that were disabled for being irrelevant to viz are not
actually that irrelevant; they just need some minor adjustments.

- RenderWidgetHostViewAuraSurfaceSynchronizationTest.DiscardDelegatedFrames
- RenderWidgetHostViewAuraSurfaceSynchronizationTest.DropFallbackWhenHidden
- RenderWidgetHostViewAuraSurfaceSynchronizationTest.SurfaceChanges
- RenderWidgetHostViewAuraTest.DiscardDelegatedFramesWithLocking
- RenderWidgetHostViewAuraTest.DiscardDelegatedFramesWithMemoryPressure
- RenderWidgetHostViewChildFrameTest.SwapCompositorFrame

Bug: 844469
Change-Id: Iddc1fdf6ebdb984afa277d537b326e7005ab1777
Reviewed-on: https://chromium-review.googlesource.com/1105312Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568301}

content/browser/renderer_host/render_widget_host_view_aura.h

@@ -404,6 +404,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
                            VirtualKeyboardFocusEnsureCaretInRect);
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraTest,
                            HitTestRegionListSubmitted);
+  FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraTest,
+                           DiscardDelegatedFramesWithMemoryPressure);
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraKeyboardTest,
                            KeyboardObserverDestroyed);
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
@@ -429,6 +431,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
                            WebContentsViewReparent);
   FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
                            TakeFallbackContent);
+  FRIEND_TEST_ALL_PREFIXES(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
+                           DiscardDelegatedFrames);
 
   class WindowObserver;
   friend class WindowObserver;

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

@@ -3672,13 +3672,6 @@ TEST_F(RenderWidgetHostViewAuraTest, OutputSurfaceIdChange) {
 // then the fallback is dropped.
 TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
        DropFallbackWhenHidden) {
-  // TODO(jonross): Delete this test once Viz launches as it will be obsolete.
-  // https://crbug.com/844469
-  if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor) ||
-      !features::IsAshInBrowserProcess()) {
-    return;
-  }
-
   view_->InitAsChild(nullptr);
   aura::client::ParentWindowWithContext(
       view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(),
@@ -3693,24 +3686,16 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
   EXPECT_FALSE(view_->HasPrimarySurface());
 
   // Submitting a CompositorFrame should not update the fallback SurfaceId
-  view_->SubmitCompositorFrame(
-      kArbitraryLocalSurfaceId,
-      MakeDelegatedFrame(1.f, gfx::Size(400, 400), gfx::Rect(400, 400)),
-      base::nullopt);
+  viz::SurfaceId surface_id(view_->GetFrameSinkId(), kArbitraryLocalSurfaceId);
+  view_->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id, 1.f, gfx::Size(400, 400)));
   EXPECT_FALSE(view_->HasPrimarySurface());
   EXPECT_FALSE(view_->HasFallbackSurface());
 }
 
 // This test verifies that the primary SurfaceId is populated on resize and
-// the fallback SurfaceId is populated on SubmitCompositorFrame.
+// the fallback SurfaceId is populated in OnFirstSurfaceActivation.
 TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest, SurfaceChanges) {
-  // TODO(jonross): Delete this test once Viz launches as it will be obsolete.
-  // https://crbug.com/844469
-  if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor) ||
-      !features::IsAshInBrowserProcess()) {
-    return;
-  }
-
   view_->InitAsChild(nullptr);
   aura::client::ParentWindowWithContext(
       view_->GetNativeView(), parent_view_->GetNativeView()->GetRootWindow(),
@@ -3732,12 +3717,14 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest, SurfaceChanges) {
   EXPECT_EQ(gfx::Size(400, 400),
             view_->delegated_frame_host_->CurrentFrameSizeInDipForTesting());
 
+  // Fallback SurfaceId should be updated in OnFirstSurfaceActivation.
   // Submitting a CompositorFrame should update the fallback SurfaceId
-  view_->SubmitCompositorFrame(
-      kArbitraryLocalSurfaceId,
-      MakeDelegatedFrame(1.f, gfx::Size(400, 400), gfx::Rect(400, 400)),
-      base::nullopt);
+  viz::SurfaceId surface_id(view_->GetFrameSinkId(),
+                            view_->GetLocalSurfaceId());
+  view_->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id, 1.f, gfx::Size(400, 400)));
   EXPECT_EQ(gfx::Size(400, 400), view_->window_->layer()->size());
+  EXPECT_EQ(surface_id, *view_->window_->layer()->GetFallbackSurfaceId());
 }
 
 // This test verifies that the primary SurfaceId is updated on device scale
@@ -3809,13 +3796,6 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
 // RenderWidgetHostViewAuraTest.DiscardDelegatedFrame.
 TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
        DiscardDelegatedFrames) {
-  // TODO(jonross): Delete this test once Viz launches as it will be obsolete.
-  // https://crbug.com/844469
-  if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor) ||
-      !features::IsAshInBrowserProcess()) {
-    return;
-  }
-
   view_->InitAsChild(nullptr);
 
   size_t max_renderer_frames =
@@ -3856,9 +3836,10 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
     views[i]->Show();
     ASSERT_TRUE(views[i]->HasPrimarySurface());
     ASSERT_FALSE(views[i]->HasFallbackSurface());
-    views[i]->SubmitCompositorFrame(
-        views[i]->GetLocalSurfaceId(),
-        MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+    viz::SurfaceId surface_id(views[i]->GetFrameSinkId(),
+                              views[i]->GetLocalSurfaceId());
+    views[i]->delegated_frame_host_->OnFirstSurfaceActivation(
+        viz::SurfaceInfo(surface_id, 1.f, frame_size));
     ASSERT_TRUE(views[i]->HasPrimarySurface());
     EXPECT_TRUE(views[i]->HasFallbackSurface());
     views[i]->Hide();
@@ -3876,9 +3857,10 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
   EXPECT_TRUE(views[1]->HasFallbackSurface());
 
   // Swap a frame on it, it should evict the next LRU [1].
-  views[0]->SubmitCompositorFrame(
-      views[0]->GetLocalSurfaceId(),
-      MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+  viz::SurfaceId surface_id0(views[0]->GetFrameSinkId(),
+                             views[0]->GetLocalSurfaceId());
+  views[0]->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id0, 1.f, frame_size));
   EXPECT_TRUE(views[0]->HasFallbackSurface());
   EXPECT_FALSE(views[1]->HasFallbackSurface());
   views[0]->Hide();
@@ -3886,9 +3868,10 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
   // LRU renderer is [1], which is still hidden. Showing it and submitting a
   // CompositorFrame to it should evict the next LRU [2].
   views[1]->Show();
-  views[1]->SubmitCompositorFrame(
-      views[1]->GetLocalSurfaceId(),
-      MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+  viz::SurfaceId surface_id1(views[1]->GetFrameSinkId(),
+                             views[1]->GetLocalSurfaceId());
+  views[1]->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id1, 1.f, frame_size));
   EXPECT_TRUE(views[0]->HasFallbackSurface());
   EXPECT_TRUE(views[1]->HasFallbackSurface());
   EXPECT_FALSE(views[2]->HasFallbackSurface());
@@ -3901,25 +3884,24 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
     views[i]->Show();
     // The renderers who don't have a frame should be waiting. The ones that
     // have a frame should not.
-    views[i]->SubmitCompositorFrame(
-        views[i]->GetLocalSurfaceId(),
-        MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+    viz::SurfaceId surface_id(views[i]->GetFrameSinkId(),
+                              views[i]->GetLocalSurfaceId());
+    views[i]->delegated_frame_host_->OnFirstSurfaceActivation(
+        viz::SurfaceInfo(surface_id, 1.f, frame_size));
     EXPECT_TRUE(views[i]->HasFallbackSurface());
   }
   EXPECT_FALSE(views[0]->HasFallbackSurface());
 
   // Swap a frame on [0], it should be evicted immediately.
-  views[0]->SubmitCompositorFrame(
-      views[0]->GetLocalSurfaceId(),
-      MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+  views[0]->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id0, 1.f, frame_size));
   EXPECT_FALSE(views[0]->HasFallbackSurface());
 
   // Make [0] visible, and swap a frame on it. Nothing should be evicted
   // although we're above the limit.
   views[0]->Show();
-  views[0]->SubmitCompositorFrame(
-      views[0]->GetLocalSurfaceId(),
-      MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+  views[0]->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id0, 1.f, frame_size));
   for (size_t i = 0; i < renderer_count; ++i)
     EXPECT_TRUE(views[i]->HasFallbackSurface()) << i;
 
@@ -3940,9 +3922,8 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
   EXPECT_FALSE(views[1]->HasFallbackSurface());
   // Show it, it should block until we give it a frame.
   views[1]->Show();
-  views[1]->SubmitCompositorFrame(
-      views[1]->GetLocalSurfaceId(),
-      MakeDelegatedFrame(1.f, size2, gfx::Rect(size2)), base::nullopt);
+  views[1]->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id1, 1.f, frame_size));
 
   for (size_t i = 0; i < renderer_count; ++i) {
     views[i]->Destroy();
@@ -3951,13 +3932,6 @@ TEST_F(RenderWidgetHostViewAuraSurfaceSynchronizationTest,
 }
 
 TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithLocking) {
-  // TODO(jonross): Delete this test once Viz launches as it will be obsolete.
-  // https://crbug.com/844469
-  if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor) ||
-      !features::IsAshInBrowserProcess()) {
-    return;
-  }
-
   view_->InitAsChild(nullptr);
 
   size_t max_renderer_frames =
@@ -3993,10 +3967,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithLocking) {
   // occur because all frames are visible.
   for (size_t i = 0; i < renderer_count; ++i) {
     views[i]->Show();
-    views[i]->SubmitCompositorFrame(
-        i ? parent_local_surface_id_allocator_.GenerateId()
-          : kArbitraryLocalSurfaceId,
-        MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+    viz::SurfaceId surface_id(views[i]->GetFrameSinkId(),
+                              views[i]->GetLocalSurfaceId());
+    views[i]->delegated_frame_host_->OnFirstSurfaceActivation(
+        viz::SurfaceInfo(surface_id, 1.f, frame_size));
     EXPECT_TRUE(views[i]->HasFallbackSurface());
   }
 
@@ -4006,9 +3980,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithLocking) {
 
   // If we lock [0] before hiding it, then [0] should not be evicted.
   views[0]->Show();
-  views[0]->SubmitCompositorFrame(
-      kArbitraryLocalSurfaceId, MakeDelegatedFrame(1.f, frame_size, view_rect),
-      base::nullopt);
+  viz::SurfaceId surface_id(views[0]->GetFrameSinkId(),
+                            views[0]->GetLocalSurfaceId());
+  views[0]->delegated_frame_host_->OnFirstSurfaceActivation(
+      viz::SurfaceInfo(surface_id, 1.f, frame_size));
   EXPECT_TRUE(views[0]->HasFallbackSurface());
   views[0]->GetDelegatedFrameHost()->LockResources();
   views[0]->Hide();
@@ -4027,13 +4002,6 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithLocking) {
 // Test that changing the memory pressure should delete saved frames. This test
 // only applies to ChromeOS.
 TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithMemoryPressure) {
-  // TODO(jonross): Delete this test once Viz launches as it will be obsolete.
-  // https://crbug.com/844469
-  if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor) ||
-      !features::IsAshInBrowserProcess()) {
-    return;
-  }
-
   view_->InitAsChild(nullptr);
 
   // The test logic below relies on having max_renderer_frames > 2.  By default,
@@ -4074,9 +4042,10 @@ TEST_F(RenderWidgetHostViewAuraTest, DiscardDelegatedFramesWithMemoryPressure) {
   // occur because all frames are visible.
   for (size_t i = 0; i < renderer_count; ++i) {
     views[i]->Show();
-    views[i]->SubmitCompositorFrame(
-        kArbitraryLocalSurfaceId,
-        MakeDelegatedFrame(1.f, frame_size, view_rect), base::nullopt);
+    viz::SurfaceId surface_id(views[i]->GetFrameSinkId(),
+                              kArbitraryLocalSurfaceId);
+    views[i]->delegated_frame_host_->OnFirstSurfaceActivation(
+        viz::SurfaceInfo(surface_id, 1.f, frame_size));
     EXPECT_TRUE(views[i]->HasFallbackSurface());
   }
 

content/browser/renderer_host/render_widget_host_view_child_frame_unittest.cc

@@ -197,44 +197,22 @@ TEST_F(RenderWidgetHostViewChildFrameTest, VisibilityTest) {
   ASSERT_FALSE(view_->IsShowing());
 }
 
-// Verify that SubmitCompositorFrame behavior is correct when a delegated
-// frame is received from a renderer process.
-TEST_F(RenderWidgetHostViewChildFrameTest, SwapCompositorFrame) {
-  // TODO(jonross): Delete this test once Viz launches as it will be obsolete.
-  // https://crbug.com/844469
-  if (base::FeatureList::IsEnabled(features::kVizDisplayCompositor) ||
-      !features::IsAshInBrowserProcess()) {
-    return;
-  }
-
+// Verify that RenderWidgetHostViewChildFrame passes the child's SurfaceId to
+// FrameConnectorDelegate to be sent to the embedding renderer.
+TEST_F(RenderWidgetHostViewChildFrameTest, PassesSurfaceId) {
   gfx::Size view_size(100, 100);
   gfx::Rect view_rect(view_size);
   float scale_factor = 1.f;
-  viz::LocalSurfaceId local_surface_id(1, base::UnguessableToken::Create());
 
   view_->SetSize(view_size);
   view_->Show();
 
-  view_->SubmitCompositorFrame(
-      local_surface_id,
-      CreateDelegatedFrame(scale_factor, view_size, view_rect), base::nullopt);
-
-  viz::SurfaceId id = GetSurfaceId();
-  if (id.is_valid()) {
-#if !defined(OS_ANDROID)
-    ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
-    viz::SurfaceManager* manager = factory->GetContextFactoryPrivate()
-                                       ->GetFrameSinkManager()
-                                       ->surface_manager();
-    viz::Surface* surface = manager->GetSurfaceForId(id);
-    EXPECT_TRUE(surface);
-#endif
+  viz::SurfaceId surface_id(view_->GetFrameSinkId(),
+                            view_->GetLocalSurfaceId());
+  viz::SurfaceInfo surface_info(surface_id, scale_factor, view_size);
+  view_->OnFirstSurfaceActivation(surface_info);
 
-    // Surface ID should have been passed to FrameConnectorDelegate to
-    // be sent to the embedding renderer.
-    EXPECT_EQ(viz::SurfaceInfo(id, scale_factor, view_size),
-              test_frame_connector_->last_surface_info_);
-  }
+  EXPECT_EQ(surface_info, test_frame_connector_->last_surface_info_);
 }
 
 // Tests that the viewport intersection rect is dispatched to the RenderWidget