Thumbnails: Capture just before navigating away

Hidden behind new feature CaptureThumbnailOnNavigatingAway.

This is very similar to what NavigationEntryScreenshotManager does.
Hopefully this is a better time for capturing screenshots than when a
page load finishes (see crbug.com/737396 and crbug.com/741856).

Bug: 718413
Change-Id: I0fbcc7964b9f89c7ce589a197ac85b85a41385ba
Reviewed-on: https://chromium-review.googlesource.com/570318Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486729}

chrome/browser/about_flags.cc

@@ -3182,6 +3182,12 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kAsyncImageDecodingDescription, kOsAll,
      MULTI_VALUE_TYPE(kAsyncImageDecodingChoices)},
 
+    {"capture-thumbnail-on-navigating-away",
+     flag_descriptions::kCaptureThumbnailOnNavigatingAwayName,
+     flag_descriptions::kCaptureThumbnailOnNavigatingAwayDescription,
+     kOsDesktop,
+     FEATURE_VALUE_TYPE(features::kCaptureThumbnailOnNavigatingAway)},
+
     // NOTE: Adding new command-line switches requires adding corresponding
     // entries to enum "LoginCustomFlags" in histograms/enums.xml. See note in
     // enums.xml and don't forget to run AboutFlagsHistogramTest unit test.

chrome/browser/flag_descriptions.cc

@@ -87,6 +87,13 @@ const char kCaptureThumbnailOnLoadFinishedDescription[] =
     "Capture a page thumbnail (for use on the New Tab page) when the page load "
     "finishes, in addition to other times a thumbnail may be captured.";
 
+const char kCaptureThumbnailOnNavigatingAwayName[] =
+    "Capture page thumbnail on navigating away";
+const char kCaptureThumbnailOnNavigatingAwayDescription[] =
+    "Capture a page thumbnail (for use on the New Tab page) when navigating "
+    "away from the current page, in addition to other times a thumbnail may be "
+    "captured.";
+
 const char kCastStreamingHwEncodingName[] =
     "Cast Streaming hardware video encoding";
 const char kCastStreamingHwEncodingDescription[] =

chrome/browser/flag_descriptions.h

@@ -78,6 +78,9 @@ extern const char kBypassAppBannerEngagementChecksDescription[];
 extern const char kCaptureThumbnailOnLoadFinishedName[];
 extern const char kCaptureThumbnailOnLoadFinishedDescription[];
 
+extern const char kCaptureThumbnailOnNavigatingAwayName[];
+extern const char kCaptureThumbnailOnNavigatingAwayDescription[];
+
 extern const char kCastStreamingHwEncodingName[];
 extern const char kCastStreamingHwEncodingDescription[];
 

chrome/browser/thumbnails/thumbnail_tab_helper.cc

@@ -54,24 +54,25 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(ThumbnailTabHelper);
 
 // Overview
 // --------
-// This class provides a service for updating thumbnails to be used in
-// "Most visited" section of the new tab page. The service can be started
-// by UpdateThumbnailIfNecessary(). The current algorithm of the service is as
-// simple as follows:
-//
-//    When a renderer is about to be hidden (this usually occurs when the
-//    current tab is closed or another tab is clicked), update the
-//    thumbnail for the tab rendered by the renderer, if needed. The
-//    heuristics to judge whether or not to update the thumbnail is
-//    implemented in ThumbnailService::ShouldAcquirePageThumbnail().
-//    If features::kCaptureThumbnailOnLoadFinished is enabled, then a thumbnail
-//    may also be captured when a page load finishes (subject to the same
-//    heuristics).
+// This class provides a service for updating thumbnails to be used in the
+// "Most visited" section of the New Tab page. The process is started by
+// UpdateThumbnailIfNecessary(), which updates the thumbnail for the current
+// tab if needed. The heuristics to judge whether to update the thumbnail are
+// implemented in ThumbnailService::ShouldAcquirePageThumbnail().
+// There are several triggers that can start the process:
+// - When a renderer is about to be hidden (this usually occurs when the current
+//   tab is closed or another tab is clicked).
+// - If features::kCaptureThumbnailOnLoadFinished is enabled: When a page load
+//   finishes.
+// - If features::kCaptureThumbnailOnNavigatingAway is enabled: Just before
+//   navigating away from the current page.
 
 ThumbnailTabHelper::ThumbnailTabHelper(content::WebContents* contents)
     : content::WebContentsObserver(contents),
       capture_on_load_finished_(base::FeatureList::IsEnabled(
           features::kCaptureThumbnailOnLoadFinished)),
+      capture_on_navigating_away_(base::FeatureList::IsEnabled(
+          features::kCaptureThumbnailOnNavigatingAway)),
       page_transition_(ui::PAGE_TRANSITION_LINK),
       load_interrupted_(false),
       weak_factory_(this) {
@@ -125,6 +126,12 @@ void ThumbnailTabHelper::DidStartNavigation(
       navigation_handle->IsSameDocument()) {
     return;
   }
+  if (capture_on_navigating_away_) {
+    // At this point, the new navigation has just been started, but the
+    // WebContents still shows the previous page. Grab a thumbnail before it
+    // goes away.
+    UpdateThumbnailIfNecessary();
+  }
   // Reset the page transition to some uninteresting type, since the actual
   // type isn't available at this point. We'll get it in DidFinishNavigation
   // (if that happens, which isn't guaranteed).
@@ -142,6 +149,8 @@ void ThumbnailTabHelper::DidFinishNavigation(
 }
 
 void ThumbnailTabHelper::DidStartLoading() {
+  // TODO(treib): We should probably track whether the load *finished* - as it
+  // is, an in-progress load will count as not interrupted.
   load_interrupted_ = false;
 }
 
@@ -170,12 +179,14 @@ void ThumbnailTabHelper::UpdateThumbnailIfNecessary() {
   if (!web_contents() || web_contents()->IsBeingDestroyed()) {
     return;
   }
-  // Skip if a pending entry exists. WidgetHidden can be called while navigating
-  // pages and this is not a time when thumbnails should be generated.
-  if (web_contents()->GetController().GetPendingEntry()) {
+
+  // Note: Do *not* use GetLastVisibleURL - it might already have been updated
+  // for a new pending navigation. The committed URL is the one corresponding
+  // to the currently visible content.
+  const GURL& url = web_contents()->GetLastCommittedURL();
+  if (!url.is_valid()) {
     return;
   }
-  const GURL& url = web_contents()->GetURL();
   Profile* profile =
       Profile::FromBrowserContext(web_contents()->GetBrowserContext());
 
@@ -183,7 +194,7 @@ void ThumbnailTabHelper::UpdateThumbnailIfNecessary() {
       ThumbnailServiceFactory::GetForProfile(profile);
 
   // Skip if we don't need to update the thumbnail.
-  if (!thumbnail_service.get() ||
+  if (!thumbnail_service ||
       !thumbnail_service->ShouldAcquirePageThumbnail(url, page_transition_)) {
     return;
   }
@@ -294,5 +305,10 @@ void ThumbnailTabHelper::RenderViewHostCreated(
 }
 
 void ThumbnailTabHelper::WidgetHidden(content::RenderWidgetHost* widget) {
+  // Skip if a pending entry exists. WidgetHidden can be called while navigating
+  // pages and this is not a time when thumbnails should be generated.
+  if (!web_contents() || web_contents()->GetController().GetPendingEntry()) {
+    return;
+  }
   UpdateThumbnailIfNecessary();
 }

chrome/browser/thumbnails/thumbnail_tab_helper.h

@@ -73,6 +73,7 @@ class ThumbnailTabHelper
   void WidgetHidden(content::RenderWidgetHost* widget);
 
   const bool capture_on_load_finished_;
+  const bool capture_on_navigating_away_;
 
   content::NotificationRegistrar registrar_;
 

chrome/common/chrome_features.cc

@@ -117,6 +117,11 @@ const base::Feature kCaptureThumbnailDependingOnTransitionType{
 const base::Feature kCaptureThumbnailOnLoadFinished{
     "CaptureThumbnailOnLoadFinished", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Whether to capture page thumbnails when navigating away from the current page
+// (in addition to any other times this might happen).
+const base::Feature kCaptureThumbnailOnNavigatingAway{
+    "CaptureThumbnailOnNavigatingAway", base::FEATURE_DISABLED_BY_DEFAULT};
+
 // Whether to trigger app banner installability checks on page load.
 const base::Feature kCheckInstallabilityForBannerOnLoad{
     "CheckInstallabilityForBannerOnLoad", base::FEATURE_DISABLED_BY_DEFAULT};

chrome/common/chrome_features.h

@@ -65,6 +65,8 @@ extern const base::Feature kCaptureThumbnailDependingOnTransitionType;
 
 extern const base::Feature kCaptureThumbnailOnLoadFinished;
 
+extern const base::Feature kCaptureThumbnailOnNavigatingAway;
+
 extern const base::Feature kCheckInstallabilityForBannerOnLoad;
 
 extern const base::Feature kClickToOpenPDFPlaceholder;

tools/metrics/histograms/enums.xml

@@ -23292,6 +23292,7 @@ from previous Chrome versions.
   <int value="835018878" label="disable-quic"/>
   <int value="838887742" label="manual-enhanced-bookmarks"/>
   <int value="841343322" label="disable-new-korean-ime"/>
+  <int value="842432903" label="CaptureThumbnailOnNavigatingAway:enabled"/>
   <int value="846951416" label="CopylessPaste:enabled"/>
   <int value="851085848" label="enable-settings-window"/>
   <int value="854730848" label="disable-app-info-dialog-mac"/>
@@ -23489,6 +23490,7 @@ from previous Chrome versions.
   <int value="1441897340" label="AndroidSpellCheckerNonLowEnd:enabled"/>
   <int value="1442798825" label="enable-quic"/>
   <int value="1442830837" label="MemoryAblation:disabled"/>
+  <int value="1454143461" label="CaptureThumbnailOnNavigatingAway:disabled"/>
   <int value="1454363479" label="disable-storage-manager"/>
   <int value="1458583431" label="arc-use-auth-endpoint"/>
   <int value="1459529277" label="disable-text-input-focus-manager"/>