Don't capture thumbnails for visible pages

A captured tab going into fullscreen is treated differently: instead
of taking up the entire screen, it only fills the browser's contents
container. This is to allow tabs to enter fullscreen while being
streamed without disrupting the user. For example, fullscreening a
video during capture will cause the video to fill the tab contents
rather than the whole screen.

Thumbnail capture uses this same infrastructure but shouldn't affect
any user-visible behavior. This CL prevents thumbnails from being
captured on visible tabs.

Ideally we'd be able to put a capture hold on a WebContents without
affecting fullscreen behavior, but implementing that is a larger
investment. Fixing this bug is a high priority.

Bug: 1112607
Change-Id: I58cc53b2d881bf65cf2a9cb70d1a5d1592d02ad2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354728
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800795}

chrome/browser/ui/thumbnails/thumbnail_capture_driver.cc

@@ -50,9 +50,10 @@ void ThumbnailCaptureDriver::UpdateCaptureState() {
     return;
   }
 
-  // Don't capture when the page is visible and the thumbnail is not
-  // requested.
-  if (!thumbnail_visible_ && page_visible_) {
+  // Capturing is not allowed when a page is visible. Otherwise it
+  // prevents the tab from properly entering fullscreen. See
+  // https://crbug.com/1112607
+  if (page_visible_) {
     client_->StopCapture();
     if (capture_state_ < CaptureState::kHaveFinalCapture)
       capture_state_ = CaptureState::kNoCapture;
@@ -67,7 +68,7 @@ void ThumbnailCaptureDriver::UpdateCaptureState() {
   // renderer. TODO(crbug.com/1073141): Figure out how to force-render
   // background tabs. This bug has detailed descriptions of steps we might
   // take to make capture more flexible in this area.
-  if (!thumbnail_visible_ && !page_visible_) {
+  if (!thumbnail_visible_) {
     client_->StopCapture();
     if (capture_state_ < CaptureState::kHaveFinalCapture)
       capture_state_ = CaptureState::kNoCapture;

chrome/browser/ui/thumbnails/thumbnail_capture_driver_unittest.cc

@@ -37,6 +37,7 @@ class ThumbnailCaptureDriverTest : public ::testing::Test {
 }  // namespace
 
 using ::testing::AnyNumber;
+using ::testing::AtLeast;
 using ::testing::Expectation;
 using ::testing::InSequence;
 
@@ -59,6 +60,25 @@ TEST_F(ThumbnailCaptureDriverTest,
       ThumbnailReadinessTracker::Readiness::kReadyForFinalCapture);
 }
 
+TEST_F(ThumbnailCaptureDriverTest,
+       NoCaptureWhenPageIsVisibleAndThumbnailIsRequested) {
+  EXPECT_CALL(mock_client_, RequestCapture()).Times(0);
+  EXPECT_CALL(mock_client_, StartCapture()).Times(0);
+  EXPECT_CALL(mock_client_, StopCapture()).Times(AnyNumber());
+
+  capture_driver_.UpdatePageVisibility(true);
+  capture_driver_.UpdateThumbnailVisibility(true);
+
+  // Simulate a page loading from start to finish
+  capture_driver_.UpdatePageReadiness(
+      ThumbnailReadinessTracker::Readiness::kNotReady);
+  capture_driver_.SetCanCapture(true);
+  capture_driver_.UpdatePageReadiness(
+      ThumbnailReadinessTracker::Readiness::kReadyForInitialCapture);
+  capture_driver_.UpdatePageReadiness(
+      ThumbnailReadinessTracker::Readiness::kReadyForFinalCapture);
+}
+
 TEST_F(ThumbnailCaptureDriverTest, NoCaptureWhenPageAndThumbnailAreNotVisible) {
   EXPECT_CALL(mock_client_, RequestCapture()).Times(0);
   EXPECT_CALL(mock_client_, StartCapture()).Times(0);
@@ -188,6 +208,29 @@ TEST_F(ThumbnailCaptureDriverTest, StopsOngoingCaptureWhenPageNoLongerReady) {
       ThumbnailReadinessTracker::Readiness::kNotReady);
 }
 
+TEST_F(ThumbnailCaptureDriverTest, StopsCaptureIfPageBecomesVisible) {
+  {
+    InSequence s;
+    EXPECT_CALL(mock_client_, StopCapture()).Times(AnyNumber());
+    EXPECT_CALL(mock_client_, RequestCapture());
+    EXPECT_CALL(mock_client_, StartCapture());
+    EXPECT_CALL(mock_client_, StopCapture()).Times(AtLeast(1));
+  }
+
+  capture_driver_.UpdateThumbnailVisibility(true);
+  capture_driver_.UpdatePageVisibility(false);
+
+  capture_driver_.UpdatePageReadiness(
+      ThumbnailReadinessTracker::Readiness::kNotReady);
+  capture_driver_.UpdatePageReadiness(
+      ThumbnailReadinessTracker::Readiness::kReadyForInitialCapture);
+  capture_driver_.SetCanCapture(true);
+
+  capture_driver_.UpdatePageVisibility(true);
+  capture_driver_.UpdatePageReadiness(
+      ThumbnailReadinessTracker::Readiness::kReadyForFinalCapture);
+}
+
 TEST_F(ThumbnailCaptureDriverTest, ContinuesCaptureWhenPageBecomesFinal) {
   {
     InSequence s;

chrome/browser/ui/thumbnails/thumbnail_tab_helper.cc

@@ -162,10 +162,9 @@ class ThumbnailTabHelper::TabStateTracker
       return;
 
     visible_ = new_visible;
+    capture_driver_.UpdatePageVisibility(visible_);
     if (!visible_ && page_readiness_ == PageReadiness::kReadyForFinalCapture)
       thumbnail_tab_helper_->CaptureThumbnailOnTabHidden();
-    else
-      capture_driver_.UpdatePageVisibility(visible_);
   }
 
   void RenderViewReady() override { capture_driver_.SetCanCapture(true); }