Re-allow capture of current tab if the thumbnail is requested.

Capture was banned for current tab as a workaround for
crbug.com/1112607, but a fix for the root cause has been submitted and
is in M87.

Now the workaround - a lesser of two evils - is causing trouble and no
longer necessary (see attached bug).

Bug: 1137133
Change-Id: If00ddacf3b55d7b17d28ab5064410bbc13b7779c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463871
Auto-Submit: Dana Fried <dfried@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816304}

chrome/browser/ui/thumbnails/thumbnail_capture_driver.cc

@@ -114,23 +114,15 @@ void ThumbnailCaptureDriver::UpdateSchedulingPriority() {
     return;
   }
 
-  // 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_) {
-    scheduler_->SetTabCapturePriority(
-        this, ThumbnailScheduler::TabCapturePriority::kNone);
-    return;
-  }
-
-  // For now don't force-load background pages. This is not ideal. We would
-  // like to grab frames from background pages to make hover cards and the
-  // "Mohnstrudel" touch/tablet tabstrip more responsive by pre-loading
-  // thumbnails from those pages. However, this currently results in a number
-  // of test failures and a possible violation of an assumption made by the
-  // 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.
+  // For now don't force-load background pages, or the current page if the
+  // thumbnail isn't being requested. This is not ideal. We would like to grab
+  // frames from background pages to make hover cards and the "Mohnstrudel"
+  // touch/tablet tabstrip more responsive by pre-loading thumbnails from those
+  // pages. However, this currently results in a number of test failures and a
+  // possible violation of an assumption made by the 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_) {
     scheduler_->SetTabCapturePriority(
         this, ThumbnailScheduler::TabCapturePriority::kNone);

chrome/browser/ui/thumbnails/thumbnail_capture_driver_unittest.cc

@@ -94,33 +94,34 @@ TEST_F(ThumbnailCaptureDriverTest,
 }
 
 TEST_F(ThumbnailCaptureDriverTest,
-       NoCaptureWhenPageIsVisibleAndThumbnailIsRequested) {
-  EXPECT_CALL(mock_client_, RequestCapture()).Times(0);
+       CaptureWhenPageIsVisibleAndThumbnailIsRequested) {
+  // StopCapture() can be called unnecessarily at first.
+  Expectation stop_capture =
+      EXPECT_CALL(mock_client_, StopCapture()).Times(AnyNumber());
+
+  // Capture shouldn't start, just requested.
   EXPECT_CALL(mock_client_, StartCapture()).Times(0);
-  EXPECT_CALL(mock_client_, StopCapture()).Times(AnyNumber());
 
+  // Simulate the current page having its thumbnail requested.
   capture_driver_.UpdatePageVisibility(true);
   capture_driver_.UpdateThumbnailVisibility(true);
 
-  EXPECT_EQ(scheduler_.priority(),
-            ThumbnailScheduler::TabCapturePriority::kNone);
-
-  // Simulate a page loading from start to finish
+  // Page becomes sufficiently loaded for capture, but no further, and
+  // the client never reports it's ready to capture. This should trigger
+  // a RequestCapture() call but nothing more.
   capture_driver_.UpdatePageReadiness(
       ThumbnailReadinessTracker::Readiness::kNotReady);
-  capture_driver_.SetCanCapture(true);
-  EXPECT_EQ(scheduler_.priority(),
-            ThumbnailScheduler::TabCapturePriority::kNone);
 
   capture_driver_.UpdatePageReadiness(
       ThumbnailReadinessTracker::Readiness::kReadyForInitialCapture);
   EXPECT_EQ(scheduler_.priority(),
-            ThumbnailScheduler::TabCapturePriority::kNone);
+            ThumbnailScheduler::TabCapturePriority::kLow);
 
-  capture_driver_.UpdatePageReadiness(
-      ThumbnailReadinessTracker::Readiness::kReadyForFinalCapture);
-  EXPECT_EQ(scheduler_.priority(),
-            ThumbnailScheduler::TabCapturePriority::kNone);
+  // Capture should only be requested when the scheduler allows it.
+  // Additionally ensure the RequestCapture() call is ordered before any
+  // StopCapture() calls.
+  EXPECT_CALL(mock_client_, RequestCapture()).Times(1).After(stop_capture);
+  capture_driver_.SetCapturePermittedByScheduler(true);
 }
 
 TEST_F(ThumbnailCaptureDriverTest, NoCaptureWhenPageAndThumbnailAreNotVisible) {
@@ -346,13 +347,13 @@ TEST_F(ThumbnailCaptureDriverTest, StopsOngoingCaptureWhenPageNoLongerReady) {
   capture_driver_.SetCapturePermittedByScheduler(false);
 }
 
-TEST_F(ThumbnailCaptureDriverTest, StopsCaptureIfPageBecomesVisible) {
+TEST_F(ThumbnailCaptureDriverTest, CanContinueCaptureIfPageBecomesVisible) {
   {
     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));
+    EXPECT_CALL(mock_client_, StopCapture()).Times(0);
   }
 
   capture_driver_.UpdateThumbnailVisibility(true);
@@ -367,13 +368,12 @@ TEST_F(ThumbnailCaptureDriverTest, StopsCaptureIfPageBecomesVisible) {
 
   capture_driver_.UpdatePageVisibility(true);
   EXPECT_EQ(scheduler_.priority(),
-            ThumbnailScheduler::TabCapturePriority::kNone);
-  capture_driver_.SetCapturePermittedByScheduler(false);
+            ThumbnailScheduler::TabCapturePriority::kLow);
 
   capture_driver_.UpdatePageReadiness(
       ThumbnailReadinessTracker::Readiness::kReadyForFinalCapture);
   EXPECT_EQ(scheduler_.priority(),
-            ThumbnailScheduler::TabCapturePriority::kNone);
+            ThumbnailScheduler::TabCapturePriority::kHigh);
 }
 
 TEST_F(ThumbnailCaptureDriverTest, ContinuesCaptureWhenPageBecomesFinal) {

chrome/browser/ui/thumbnails/thumbnail_tab_helper.cc

@@ -184,7 +184,7 @@ class ThumbnailTabHelper::TabStateTracker
 
     visible_ = new_visible;
     capture_driver_.UpdatePageVisibility(visible_);
-    if (!visible_ && page_readiness_ == PageReadiness::kReadyForFinalCapture)
+    if (!visible_ && page_readiness_ != PageReadiness::kNotReady)
       thumbnail_tab_helper_->CaptureThumbnailOnTabHidden();
   }