Extract capture state machine

This moves much of the capturing logic to ThumbnailCaptureDriver which
doesn't depend on WebContents or video capture details. Instead, it
expects calls from its client on state changes and calls back into a
client interface to request, start, and stop capture.

Since the new class is testable, this also adds unit tests.

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

chrome/browser/ui/BUILD.gn

@@ -1239,6 +1239,8 @@ static_library("ui") {
       "task_manager/task_manager_columns.h",
       "task_manager/task_manager_table_model.cc",
       "task_manager/task_manager_table_model.h",
+      "thumbnails/thumbnail_capture_driver.cc",
+      "thumbnails/thumbnail_capture_driver.h",
       "thumbnails/thumbnail_image.cc",
       "thumbnails/thumbnail_image.h",
       "thumbnails/thumbnail_readiness_tracker.cc",

chrome/browser/ui/thumbnails/thumbnail_capture_driver.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/thumbnails/thumbnail_capture_driver.h"
+
+#include "base/logging.h"
+
+// static
+constexpr base::TimeDelta ThumbnailCaptureDriver::kCooldownDelay;
+
+// static
+constexpr size_t ThumbnailCaptureDriver::kMaxCooldownRetries;
+
+ThumbnailCaptureDriver::ThumbnailCaptureDriver(Client* client)
+    : client_(client) {}
+
+ThumbnailCaptureDriver::~ThumbnailCaptureDriver() = default;
+
+void ThumbnailCaptureDriver::UpdatePageReadiness(PageReadiness page_readiness) {
+  page_readiness_ = page_readiness;
+  UpdateCaptureState();
+}
+
+void ThumbnailCaptureDriver::UpdatePageVisibility(bool page_visible) {
+  page_visible_ = page_visible;
+  UpdateCaptureState();
+}
+
+void ThumbnailCaptureDriver::UpdateThumbnailVisibility(bool thumbnail_visible) {
+  thumbnail_visible_ = thumbnail_visible;
+  UpdateCaptureState();
+}
+
+void ThumbnailCaptureDriver::SetCanCapture(bool can_capture) {
+  LOG(ERROR) << "SetCanCapture " << this << " " << can_capture;
+  can_capture_ = can_capture;
+  UpdateCaptureState();
+}
+
+void ThumbnailCaptureDriver::GotFrame() {
+  if (capture_state_ == CaptureState::kCooldown)
+    captured_cooldown_frame_ = true;
+}
+
+void ThumbnailCaptureDriver::UpdateCaptureState() {
+  // Stop any existing capture and return if the page is not ready.
+  if (page_readiness_ == PageReadiness::kNotReady) {
+    client_->StopCapture();
+    capture_state_ = CaptureState::kNoCapture;
+    return;
+  }
+
+  // Don't capture when the page is visible and the thumbnail is not
+  // requested.
+  if (!thumbnail_visible_ && page_visible_) {
+    client_->StopCapture();
+    if (capture_state_ < CaptureState::kHaveFinalCapture)
+      capture_state_ = CaptureState::kNoCapture;
+    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.
+  if (!thumbnail_visible_ && !page_visible_) {
+    client_->StopCapture();
+    if (capture_state_ < CaptureState::kHaveFinalCapture)
+      capture_state_ = CaptureState::kNoCapture;
+    return;
+  }
+
+  // Now we know the page is ready for capture and the thumbnail is
+  // visible.
+
+  // If the page is in its final state and we already have a good
+  // thumbnail, don't need to anything.
+  if (page_readiness_ == PageReadiness::kReadyForFinalCapture &&
+      capture_state_ == CaptureState::kHaveFinalCapture) {
+    return;
+  }
+
+  // Now we know the page is a candidate for capture.
+
+  // Request to capture if we haven't done so.
+  if (capture_state_ < CaptureState::kCaptureRequested) {
+    capture_state_ = CaptureState::kCaptureRequested;
+    client_->RequestCapture();
+  }
+
+  // Now, our |capture_state_| is at least |CaptureState::kCaptureRequested|.
+
+  // Wait until our client is able to capture.
+  if (!can_capture_) {
+    // It is possible we were actively capturing and the client reported
+    // it can no longer capture. Reset our state to re-request capture
+    // later.
+    capture_state_ = CaptureState::kCaptureRequested;
+    cooldown_timer_.AbandonAndStop();
+    return;
+  }
+
+  // The client is ready so start capturing. Continue below in case the
+  // page is fully loaded, in which case we will wrap things up
+  // immediately.
+  if (capture_state_ == CaptureState::kCaptureRequested) {
+    capture_state_ = CaptureState::kCapturing;
+    client_->StartCapture();
+  }
+
+  // If the page is finalized, enter cooldown if we haven't yet.
+  if (page_readiness_ == PageReadiness::kReadyForFinalCapture &&
+      capture_state_ == CaptureState::kCapturing) {
+    StartCooldown();
+    return;
+  }
+
+  // If the page is finalized and we are in cooldown capture mode, we
+  // don't need to do anything. The cooldown timer callback will
+  // finalize everything.
+  if (page_readiness_ == PageReadiness::kReadyForFinalCapture &&
+      capture_state_ == CaptureState::kCooldown) {
+    return;
+  }
+
+  // If we aren't actively capturing, we should've handled this above.
+  DCHECK_EQ(capture_state_, CaptureState::kCapturing)
+      << "page_readiness_ = " << static_cast<int>(page_readiness_);
+}
+
+void ThumbnailCaptureDriver::StartCooldown() {
+  DCHECK_EQ(page_readiness_, PageReadiness::kReadyForFinalCapture);
+  DCHECK_EQ(capture_state_, CaptureState::kCapturing);
+
+  capture_state_ = CaptureState::kCooldown;
+  captured_cooldown_frame_ = false;
+  cooldown_retry_count_ = 0U;
+
+  if (cooldown_timer_.IsRunning()) {
+    cooldown_timer_.Reset();
+  } else {
+    cooldown_timer_.Start(
+        FROM_HERE, kCooldownDelay,
+        base::BindRepeating(&ThumbnailCaptureDriver::OnCooldownEnded,
+                            weak_ptr_factory_.GetWeakPtr()));
+  }
+}
+
+void ThumbnailCaptureDriver::OnCooldownEnded() {
+  if (capture_state_ < CaptureState::kCooldown)
+    return;
+
+  if (!captured_cooldown_frame_ &&
+      cooldown_retry_count_ < kMaxCooldownRetries) {
+    ++cooldown_retry_count_;
+    cooldown_timer_.Reset();
+    return;
+  }
+
+  capture_state_ = CaptureState::kHaveFinalCapture;
+  client_->StopCapture();
+}

chrome/browser/ui/thumbnails/thumbnail_capture_driver.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_CAPTURE_DRIVER_H_
+#define CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_CAPTURE_DRIVER_H_
+
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/ui/thumbnails/thumbnail_readiness_tracker.h"
+
+class ThumbnailCaptureDriver {
+ public:
+  class Client {
+   public:
+    // Asks the client to prepare for capture. The client should reply
+    // by calling SetCanCapture(true) when ready.
+    virtual void RequestCapture() = 0;
+
+    // Begin capturing and updating the thumbnail immediately. This will
+    // not be called unless SetCanCapture(true) was called. The client
+    // should call GotFrame() whenever it gets a new frame from capture.
+    virtual void StartCapture() = 0;
+
+    // Stop capturing and cancel previous RequestCapture() call.
+    virtual void StopCapture() = 0;
+
+   protected:
+    // Not deleted through interface pointer.
+    ~Client() = default;
+  };
+
+  using PageReadiness = ThumbnailReadinessTracker::Readiness;
+
+  explicit ThumbnailCaptureDriver(Client* client);
+  ~ThumbnailCaptureDriver();
+
+  // Update the capture state machine with new data.
+  void UpdatePageReadiness(PageReadiness page_readiness);
+  void UpdatePageVisibility(bool page_visible);
+  void UpdateThumbnailVisibility(bool thumbnail_visible);
+
+  // Can be called whenever. Will not issue a Client::StartCapture()
+  // call if this is false. If set to false during capture, assumes the
+  // capture stopped but the request is still outstanding. On the next
+  // call with true, may immediately issue a Client::StartCapture()
+  // call.
+  void SetCanCapture(bool can_capture);
+
+  // Notify scheduler a frame was received during capture.
+  void GotFrame();
+
+  // Determines how long to wait for final capture, and how many times
+  // to retry if one is not received. Exposed for testing.
+  static constexpr base::TimeDelta kCooldownDelay =
+      base::TimeDelta::FromMilliseconds(500);
+  static constexpr size_t kMaxCooldownRetries = 3;
+
+ private:
+  // How far along we are in the capture lifecycle for a given page.
+  enum class CaptureState : int {
+    // We have not started capturing the current page.
+    kNoCapture = 0,
+    // We have asked to capture but haven't started yet.
+    kCaptureRequested,
+    // We are currently capturing.
+    kCapturing,
+    // We are attempting to capture our last frame.
+    kCooldown,
+    // We have a good capture of the final page.
+    kHaveFinalCapture,
+  };
+
+  void UpdateCaptureState();
+  void StartCooldown();
+  void OnCooldownEnded();
+
+  Client* const client_;
+
+  PageReadiness page_readiness_ = PageReadiness::kNotReady;
+  bool page_visible_ = false;
+  bool thumbnail_visible_ = false;
+  bool can_capture_ = false;
+
+  CaptureState capture_state_ = CaptureState::kNoCapture;
+
+  // Has a frame been captured during cooldown?
+  bool captured_cooldown_frame_ = false;
+  size_t cooldown_retry_count_ = 0U;
+
+  base::RetainingOneShotTimer cooldown_timer_;
+
+  base::WeakPtrFactory<ThumbnailCaptureDriver> weak_ptr_factory_{this};
+};
+
+#endif  // CHROME_BROWSER_UI_THUMBNAILS_THUMBNAIL_CAPTURE_DRIVER_H_

chrome/test/BUILD.gn

@@ -4165,6 +4165,7 @@ test("unit_tests") {
       "../browser/ui/tabs/tab_switch_event_latency_recorder_unittest.cc",
       "../browser/ui/tabs/test_tab_strip_model_delegate.cc",
       "../browser/ui/tabs/test_tab_strip_model_delegate.h",
+      "../browser/ui/thumbnails/thumbnail_capture_driver_unittest.cc",
       "../browser/ui/thumbnails/thumbnail_image_unittest.cc",
       "../browser/ui/toolbar/app_menu_model_unittest.cc",
       "../browser/ui/toolbar/back_forward_menu_model_unittest.cc",