Refactor GrabWindowSnapshot and GrabWindowSnapshotImpl names to...

Refactor GrabWindowSnapshot and GrabWindowSnapshotImpl names to GrabWindowSnapshotForUser and GrabWindowSnapshot respectively.
Use GrabWindowSnapshot (the old internal version) in tests.


BUG=139694


Review URL: https://chromiumcodereview.appspot.com/10830158

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150085 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/ash/screenshot_taker.cc

@@ -121,9 +121,8 @@ bool GrabWindowSnapshot(aura::Window* window,
                         const gfx::Rect& snapshot_bounds,
                         std::vector<unsigned char>* png_data) {
 #if defined(OS_LINUX)
-  // browser::GrabWindowSnapshot checks this too, but RootWindow::GrabSnapshot
-  // does not. The statement below is only to support linux-specific XGetImage
-  // optimization.
+  // chrome::GrabWindowSnapshotForUser checks this too, but
+  // RootWindow::GrabSnapshot does not.
   if (AreScreenshotsDisabled())
     return false;
 
@@ -134,7 +133,7 @@ bool GrabWindowSnapshot(aura::Window* window,
     return true;
 #endif  // OS_LINUX
 
-  return chrome::GrabWindowSnapshot(window, png_data, snapshot_bounds);
+  return chrome::GrabWindowSnapshotForUser(window, png_data, snapshot_bounds);
 }
 
 // How opaque should the layer that we flash onscreen to provide visual

chrome/browser/ui/webui/feedback_ui.cc

@@ -188,8 +188,9 @@ void ShowFeedbackPage(Browser* browser,
   native_window = browser->window()->GetNativeWindow();
   snapshot_bounds = gfx::Rect(browser->window()->GetBounds().size());
 #endif
-  bool success = chrome::GrabWindowSnapshot(native_window, last_screenshot_png,
-                                            snapshot_bounds);
+  bool success = chrome::GrabWindowSnapshotForUser(native_window,
+                                                   last_screenshot_png,
+                                                   snapshot_bounds);
   FeedbackUtil::SetScreenshotSize(success ? snapshot_bounds : gfx::Rect());
 
   std::string feedback_url = std::string(chrome::kChromeUIFeedbackURL) + "?" +

chrome/browser/ui/window_snapshot/window_snapshot.cc

@@ -10,19 +10,14 @@
 
 namespace chrome {
 
-// Like GrabWindowSnapshot, but does not check if policy settings allow taking
-// screenshots. Implemented in a platform-specific way.
-bool GrabWindowSnapshotImpl(gfx::NativeWindow window,
-                            std::vector<unsigned char>* png_representation,
-                            const gfx::Rect& snapshot_bounds);
-
-bool GrabWindowSnapshot(
+bool GrabWindowSnapshotForUser(
     gfx::NativeWindow window,
     std::vector<unsigned char>* png_representation,
     const gfx::Rect& snapshot_bounds) {
   if (g_browser_process->local_state()->GetBoolean(prefs::kDisableScreenshots))
     return false;
-  return GrabWindowSnapshotImpl(window, png_representation, snapshot_bounds);
+  return internal::GrabWindowSnapshot(window, png_representation,
+        snapshot_bounds);
 }
 
 void RegisterScreenshotPrefs(PrefService* service) {

chrome/browser/ui/window_snapshot/window_snapshot.h

@@ -17,17 +17,33 @@ class Rect;
 
 namespace chrome {
 
+void RegisterScreenshotPrefs(PrefService* service);
+
 // Grabs a snapshot of the rectangle area |snapshot_bounds| with respect to the
 // top left corner of the designated window and stores a PNG representation
 // into a byte vector. On Windows, |window| may be NULL to grab a snapshot of
-// the primary monitor. Returns true if the operation is successful.
-bool GrabWindowSnapshot(
+// the primary monitor. This takes into account calling user context (ie. checks
+// policy settings if taking screenshots is allowed), and is intended to be used
+// by browser code. If you need to take a screenshot for debugging purposes,
+// consider using GrabWindowSnapshot.
+// Returns true if the operation is successful (ie. permitted).
+bool GrabWindowSnapshotForUser(
     gfx::NativeWindow window,
     std::vector<unsigned char>* png_representation,
     const gfx::Rect& snapshot_bounds);
 
-void RegisterScreenshotPrefs(PrefService* service);
+namespace internal {
+
+// Like GrabWindowSnapshotForUser, but does not perform additional security
+// checks - just grabs a snapshot. This is intended to be used for debugging
+// purposes where no BrowserProcess instance is available (ie. tests).
+// DO NOT use in a result of user action.
+bool GrabWindowSnapshot(
+    gfx::NativeWindow window,
+    std::vector<unsigned char>* png_representation,
+    const gfx::Rect& snapshot_bounds);
 
+}  // namespace internal
 }  // namespace chrome
 
 #endif  // CHROME_BROWSER_UI_WINDOW_SNAPSHOT_WINDOW_SNAPSHOT_H_

chrome/browser/ui/window_snapshot/window_snapshot_aura.cc

@@ -14,10 +14,11 @@
 #include "ui/gfx/rect.h"
 
 namespace chrome {
+namespace internal {
 
-bool GrabWindowSnapshotImpl(gfx::NativeWindow window,
-                            std::vector<unsigned char>* png_representation,
-                            const gfx::Rect& snapshot_bounds) {
+bool GrabWindowSnapshot(gfx::NativeWindow window,
+                        std::vector<unsigned char>* png_representation,
+                        const gfx::Rect& snapshot_bounds) {
   ui::Compositor* compositor = window->layer()->GetCompositor();
 
   gfx::Rect read_pixels_bounds = snapshot_bounds;
@@ -48,4 +49,5 @@ bool GrabWindowSnapshotImpl(gfx::NativeWindow window,
   return true;
 }
 
+}  // namespace internal
 }  // namespace chrome

chrome/browser/ui/window_snapshot/window_snapshot_gtk.cc

@@ -28,10 +28,11 @@ cairo_status_t SnapshotCallback(void* closure,
 }  // namespace
 
 namespace chrome {
+namespace internal {
 
-bool GrabWindowSnapshotImpl(gfx::NativeWindow window_handle,
-                            std::vector<unsigned char>* png_representation,
-                            const gfx::Rect& snapshot_bounds) {
+bool GrabWindowSnapshot(gfx::NativeWindow window_handle,
+                        std::vector<unsigned char>* png_representation,
+                        const gfx::Rect& snapshot_bounds) {
   GdkWindow* gdk_window = gtk_widget_get_window(GTK_WIDGET(window_handle));
   Display* display = GDK_WINDOW_XDISPLAY(gdk_window);
   XID win = GDK_WINDOW_XID(gdk_window);
@@ -75,4 +76,5 @@ bool GrabWindowSnapshotImpl(gfx::NativeWindow window_handle,
   return true;
 }
 
+}  // namespace internal
 }  // namespace chrome

chrome/browser/ui/window_snapshot/window_snapshot_mac.mm

@@ -12,10 +12,11 @@
 #include "ui/gfx/rect.h"
 
 namespace chrome {
+namespace internal {
 
-bool GrabWindowSnapshotImpl(gfx::NativeWindow window,
-                            std::vector<unsigned char>* png_representation,
-                            const gfx::Rect& snapshot_bounds) {
+bool GrabWindowSnapshot(gfx::NativeWindow window,
+                        std::vector<unsigned char>* png_representation,
+                        const gfx::Rect& snapshot_bounds) {
   NSScreen* screen = [[NSScreen screens] objectAtIndex:0];
   gfx::Rect screen_bounds = gfx::Rect(NSRectToCGRect([screen frame]));
   gfx::Rect window_bounds = gfx::Rect(NSRectToCGRect([window frame]));
@@ -58,4 +59,5 @@ bool GrabWindowSnapshotImpl(gfx::NativeWindow window,
   return true;
 }
 
+}  // namespace internal
 }  // namespace chrome

chrome/browser/ui/window_snapshot/window_snapshot_mac_unittest.mm

@@ -8,9 +8,6 @@
 
 #include "base/memory/scoped_nsobject.h"
 #include "base/memory/scoped_ptr.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/test/base/testing_browser_process.h"
-#include "chrome/test/base/testing_pref_service.h"
 #include "testing/platform_test.h"
 #include "ui/gfx/rect.h"
 
@@ -20,10 +17,6 @@ namespace {
 typedef PlatformTest GrabWindowSnapshotTest;
 
 TEST_F(GrabWindowSnapshotTest, TestGrabWindowSnapshot) {
-  // GrabWindowSnapshot reads local state, so set it up
-  ScopedTestingLocalState local_state(
-    static_cast<TestingBrowserProcess*>(g_browser_process));
-
   // Launch a test window so we can take a snapshot.
   NSRect frame = NSMakeRect(0, 0, 400, 400);
   scoped_nsobject<NSWindow> window(
@@ -37,7 +30,8 @@ TEST_F(GrabWindowSnapshotTest, TestGrabWindowSnapshot) {
   scoped_ptr<std::vector<unsigned char> > png_representation(
       new std::vector<unsigned char>);
   gfx::Rect bounds = gfx::Rect(0, 0, frame.size.width, frame.size.height);
-  EXPECT_TRUE(GrabWindowSnapshot(window, png_representation.get(), bounds));
+  EXPECT_TRUE(internal::GrabWindowSnapshot(window, png_representation.get(),
+                                           bounds));
 
   // Copy png back into NSData object so we can make sure we grabbed a png.
   scoped_nsobject<NSData> image_data(

chrome/browser/ui/window_snapshot/window_snapshot_win.cc

@@ -35,10 +35,11 @@ gfx::Rect GetWindowBounds(gfx::NativeWindow window_handle) {
 }  // namespace
 
 namespace chrome {
+namespace internal {
 
-bool GrabWindowSnapshotImpl(gfx::NativeWindow window_handle,
-                            std::vector<unsigned char>* png_representation,
-                            const gfx::Rect& snapshot_bounds) {
+bool GrabWindowSnapshot(gfx::NativeWindow window_handle,
+                        std::vector<unsigned char>* png_representation,
+                        const gfx::Rect& snapshot_bounds) {
   DCHECK(snapshot_bounds.right() <= GetWindowBounds(window_handle).right());
   DCHECK(snapshot_bounds.bottom() <= GetWindowBounds(window_handle).bottom());
 
@@ -97,4 +98,5 @@ bool GrabWindowSnapshotImpl(gfx::NativeWindow window_handle,
   return true;
 }
 
+}  // namespace internal
 }  // namespace chrome

chrome/test/base/ui_test_utils_win.cc

@@ -124,10 +124,10 @@ bool SaveScreenSnapshotToDirectory(const FilePath& directory,
     RECT& rect = monitor_info.rcMonitor;
 
     std::vector<unsigned char> png_data;
-    if (chrome::GrabWindowSnapshot(NULL, &png_data,
-                                   gfx::Rect(rect.right - rect.left,
-                                             rect.bottom - rect.top)) &&
-        png_data.size() <= INT_MAX) {
+    if (chrome::internal::GrabWindowSnapshot(NULL, &png_data,
+                                             gfx::Rect(rect.right - rect.left,
+                                                       rect.bottom - rect.top))
+        && png_data.size() <= INT_MAX) {
       int bytes = static_cast<int>(png_data.size());
       int written = file_util::WriteFile(
           out_path, reinterpret_cast<char*>(&png_data[0]), bytes);

chrome/test/gpu/gpu_pixel_browsertest.cc

@@ -400,7 +400,8 @@ class GpuPixelBrowserTest : public InProcessBrowserTest {
                               tab_contents_bounds.height());
 
     gfx::NativeWindow native_window = browser()->window()->GetNativeWindow();
-    if (!chrome::GrabWindowSnapshot(native_window, &png, snapshot_bounds)) {
+    if (!chrome::GrabWindowSnapshotForUser(native_window, &png,
+                                           snapshot_bounds)) {
       LOG(ERROR) << "browser::GrabWindowSnapShot() failed";
       return false;
     }

chrome/test/perf/rendering/throughput_tests.cc

@@ -230,7 +230,8 @@ class ThroughputTest : public BrowserPerfTest {
                               tab_contents_bounds.height());
 
     gfx::NativeWindow native_window = browser()->window()->GetNativeWindow();
-    if (!chrome::GrabWindowSnapshot(native_window, &png, snapshot_bounds)) {
+    if (!chrome::GrabWindowSnapshotForUser(native_window, &png,
+                                           snapshot_bounds)) {
       LOG(ERROR) << "browser::GrabWindowSnapShot() failed";
       return false;
     }

chrome_frame/test/test_with_web_server.cc

@@ -697,8 +697,7 @@ TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_XHRTest) {
 const wchar_t kInstallFlowTestUrl[] =
     L"install_flow_test.html";
 
-// crbug.com/139694
-TEST_F(ChromeFrameTestWithWebServer, DISABLED_FullTabModeIE_InstallFlowTest) {
+TEST_F(ChromeFrameTestWithWebServer, FullTabModeIE_InstallFlowTest) {
   if (base::win::GetVersion() < base::win::VERSION_VISTA) {
     ScopedChromeFrameRegistrar::UnregisterAtPath(
         GetChromeFrameBuildPath().value(),
@@ -902,8 +901,7 @@ class UaTemplateFileResponse : public test_server::FileResponse {
 //
 // This test currently fails because GCF does not add the chromeframe header
 // to requests that mshtml initiates via IInternetSession::CreateBinding.
-// crbug.com/139694
-TEST_F(ChromeFrameTestWithWebServer, DISABLED_FullTabModeIE_RefreshMshtmlTest) {
+TEST_F(ChromeFrameTestWithWebServer, FAILS_FullTabModeIE_RefreshMshtmlTest) {
   const wchar_t* kPages[] = {
     L"mshtml_refresh_test.html",
     L"mshtml_refresh_test_popup.html",