Consistent CopyFromSurface() API, consolidated to RWHV

content/browser/frame_host/navigation_entry_screenshot_manager.cc

Index: content/browser/frame_host/navigation_entry_screenshot_manager.cc
diff --git a/content/browser/frame_host/navigation_entry_screenshot_manager.cc b/content/browser/frame_host/navigation_entry_screenshot_manager.cc
index 01f90564b60794239da94522326a7aeff68fead5..f6dc6531e2ad1d9a49f1aed9582a4c94af45dbeb 100644
--- a/content/browser/frame_host/navigation_entry_screenshot_manager.cc
+++ b/content/browser/frame_host/navigation_entry_screenshot_manager.cc
@@ -113,13 +113,21 @@ void NavigationEntryScreenshotManager::ClearAllScreenshots() {
 void NavigationEntryScreenshotManager::TakeScreenshotImpl(
     RenderViewHost* host,
     NavigationEntryImpl* entry) {
-  DCHECK(host && host->GetWidget()->GetView());
+  DCHECK(host && host->GetWidget());
   DCHECK(entry);
-  host->GetWidget()->CopyFromBackingStore(
-      gfx::Rect(), host->GetWidget()->GetView()->GetViewBounds().size(),
-      base::Bind(&NavigationEntryScreenshotManager::OnScreenshotTaken,
-                 screenshot_factory_.GetWeakPtr(), entry->GetUniqueID()),
-      kAlpha_8_SkColorType);
+  if (auto* view = host->GetWidget()->GetView()) {

[alexmos, 2017/02/28 19:44:09]

Do you need this check?  It looks like
NavigationEntryScreenshotManager::TakeScreenshot already null-checks this before
calling TakeScreenshotImpl, so it doesn't seem like the else clause would ever
be reached.  If you want to keep the null case handling here, perhaps remove the
null check from TakeScreenshot?

[miu, 2017/03/01 01:17:02]

Done. Plus, things were simpler when I just dumped the remaining code at the end
of TakeScreenshot() (i.e., no need for a separate TakeScreenshotImpl() method).

[alexmos, 2017/03/01 01:30:05]

Sounds good, but looks like there's another test in
web_contents_view_aura_browsertest.cc that overrides TakeScreenshotImpl, so do
you need to fix that test as well?

[miu, 2017/03/01 21:57:18]

Ah, yes. Fixed. I had to add back a "notification" method for the test to
override.

+    // This screenshot is destined for the UI, so size the result to the actual
+    // on-screen size of the view (and not its device-rendering size).
+    const gfx::Size view_size_on_screen = view->GetViewBounds().size();
+    view->CopyFromSurface(
+        gfx::Rect(), view_size_on_screen,
+        base::Bind(&NavigationEntryScreenshotManager::OnScreenshotTaken,
+                   screenshot_factory_.GetWeakPtr(), entry->GetUniqueID()),
+        kAlpha_8_SkColorType);
+  } else {
+    OnScreenshotTaken(entry->GetUniqueID(), SkBitmap(),
+                      READBACK_SURFACE_UNAVAILABLE);
+  }
 }
 
 void NavigationEntryScreenshotManager::SetMinScreenshotIntervalMS(