Discard compositor frames from unloaded web content

content/browser/renderer_host/render_widget_host_unittest.cc

Index: content/browser/renderer_host/render_widget_host_unittest.cc
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index 972f34021b3a5ddaaa670a1a445ef43646e6fb07..c4258d27af17b5a1a84faf4410f4d32101403ed3 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -1253,7 +1253,7 @@ TEST_F(RenderWidgetHostTest, NewContentRenderingTimeout) {
       base::TimeDelta::FromMicroseconds(10));
 
   // Test immediate start and stop, ensuring that the timeout doesn't fire.
-  host_->StartNewContentRenderingTimeout();
+  host_->StartNewContentRenderingTimeout(0);
   host_->OnFirstPaintAfterLoad();
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(),
@@ -1265,7 +1265,7 @@ TEST_F(RenderWidgetHostTest, NewContentRenderingTimeout) {
   // Test that the timer doesn't fire if it receives a stop before
   // a start.
   host_->OnFirstPaintAfterLoad();
-  host_->StartNewContentRenderingTimeout();
+  host_->StartNewContentRenderingTimeout(0);
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(),
       TimeDelta::FromMicroseconds(20));
@@ -1274,7 +1274,7 @@ TEST_F(RenderWidgetHostTest, NewContentRenderingTimeout) {
   EXPECT_FALSE(host_->new_content_rendering_timeout_fired());
 
   // Test with a long delay to ensure that it does fire this time.
-  host_->StartNewContentRenderingTimeout();
+  host_->StartNewContentRenderingTimeout(0);
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(),
       TimeDelta::FromMicroseconds(20));
@@ -1282,6 +1282,28 @@ TEST_F(RenderWidgetHostTest, NewContentRenderingTimeout) {
   EXPECT_TRUE(host_->new_content_rendering_timeout_fired());
 }
 
+// This tests that a compositor frame received with a stale content source ID
+// in its metadata is properly discarded.
+TEST_F(RenderWidgetHostTest, SwapCompositorFrameWithBadSourceId) {
+  host_->StartNewContentRenderingTimeout(100);
+  host_->OnFirstPaintAfterLoad();
+
+  // First swap a frame with an invalid ID.
+  cc::CompositorFrame frame;
+  frame.metadata.content_source_id = 99;
+  host_->OnMessageReceived(ViewHostMsg_SwapCompositorFrame(
+      0, 0, frame, std::vector<IPC::Message>()));
+  EXPECT_FALSE(
+      static_cast<TestView*>(host_->GetView())->did_swap_compositor_frame());
+
+  // Test with a valid content ID as a control.
+  frame.metadata.content_source_id = 100;

[danakj, 2017/03/02 18:12:50]

The code also accepts source id > 100. I've been wondering about it, and notice
that you don't test it here. Is that > actually important, if so it should be
tested. If not, maybe removed?

[kenrb, 2017/03/02 19:35:57]

That's a point, and I've added it to the test. The > is important because at
times the first compositor frame for a newly-loaded page can arrive at the
browser before the navigation commit message. It is racy because they are sent
by different threads. The comparison accounts for this case.

[danakj, 2017/03/02 19:56:34]

OK Thanks. A comment on the >= would be appreciated so this isn't lost in
codereview.

[kenrb, 2017/03/02 21:19:51]

Done.

+  host_->OnMessageReceived(ViewHostMsg_SwapCompositorFrame(
+      0, 0, frame, std::vector<IPC::Message>()));
+  EXPECT_TRUE(
+      static_cast<TestView*>(host_->GetView())->did_swap_compositor_frame());
+}
+
 TEST_F(RenderWidgetHostTest, TouchEmulator) {
   simulated_event_time_delta_seconds_ = 0.1;
   // Immediately ack all touches instead of sending them to the renderer.