Take screenshot of active window

chrome/browser/ui/ash/chrome_screenshot_grabber.cc

 // Copyright 2014 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/ash/chrome_screenshot_grabber.h"
 
 #include <stddef.h>
 
 #include "ash/shell.h"
 #include "ash/system/system_notifier.h"
@@ skipping 332 matching lines @@
         base::FilePath());
     return;
   }
 
   base::FilePath screenshot_path =
       screenshot_directory.AppendASCII(GetScreenshotBaseFilename() + ".png");
   screenshot_grabber_->TakeScreenshot(window, rect, screenshot_path);
   content::RecordAction(base::UserMetricsAction("Screenshot_TakePartial"));
 }
 
+void ChromeScreenshotGrabber::HandleTakeWindowScreenshot(aura::Window* window) {
+  if (ScreenshotsDisabled()) {
+    screenshot_grabber_->NotifyScreenshotCompleted(
+        ui::ScreenshotGrabberObserver::SCREENSHOTS_DISABLED, base::FilePath());
+    return;
+  }
+
+  base::FilePath screenshot_directory;
+  if (!GetScreenshotDirectory(&screenshot_directory)) {
+    screenshot_grabber_->NotifyScreenshotCompleted(
+        ui::ScreenshotGrabberObserver::SCREENSHOT_GET_DIR_FAILED,
+        base::FilePath());
+    return;
+  }
+
+  base::FilePath screenshot_path =
+      screenshot_directory.AppendASCII(GetScreenshotBaseFilename() + ".png");
+  aura::Window* root_window = window->GetRootWindow();
+  gfx::Rect root_rect = root_window->bounds();

[sky, 2016/03/25 15:27:53]

This code assumes window is a direct child of root_window, that is not
necessary.

[Qiang(Joe) Xu, 2016/03/25 19:09:40]

GetRootWindow() will get root window, it is not parent() method which only gets
direct parent.

[sky, 2016/03/25 20:37:08]

GetRootWindow() walks up the tree starting at window. So, window is not
necessarily a direct child of the root.

+  screenshot_grabber_->TakeScreenshot(window, root_rect, screenshot_path);

[sky, 2016/03/25 15:27:53]

I'm confused by the coordinates here. You're passing in window to get the
screenshot, but you're passing in bounds in terms of another window. I would
expect root_rect to be gfx::Rect(window->bounds().size()); Why isn't it that?

If you do need root_rect in terms of the root_window, then you should convert to
the root_window instead of assuming window->parent() == root_window.

[Qiang(Joe) Xu, 2016/03/25 19:09:40]

Here we should use the root window's coordinates as we looked at TakeScreenshot
method, if we are using window->bounds() rect, it will use the rect based on the
window's rect, which will cause the problem in crbug/481059 comment＃20. We
should use fullscreen's rect instead, in this case, that is root window.

window->parent() doesn't sound right, 
https://code.google.com/p/chromium/codesearch#chromium/src/ui/aura/window.cc&...

I believe GetRootWindow is doing the right way, clarify me if I am wrong on
this.

[sky, 2016/03/25 20:37:08]

This is the comment says for the function you're using:
  // Takes a screenshot of |rect| in |window| in that window's coordinate space
  // saving the result to |screenshot_path|.
So again, I would expect you to use gfx::Rect(window->bounds().size());

As to the root window. If you care about the location in terms of the root, then
you should be using ConvertRectToTarget().

[Qiang(Joe) Xu, 2016/03/25 21:15:54]

I get it:
My original one is: passing active_window and root_rect
now is: passing root_window and active_window_rect
I am using window->GetBoundsInRootWindow() // Returns the window's bounds in
root window's coordinates.
this change matches well with the TakeScreenshot API.

[sky, 2016/03/25 22:58:43]

I don't think that's right either. The problem with doing that is that if
another window happens to be on top of the window you're taking a screenshot of
it'll be included in the output. I don't think that is expected.

+  content::RecordAction(base::UserMetricsAction("Screenshot_TakeWindow"));
+}
+
 bool ChromeScreenshotGrabber::CanTakeScreenshot() {
   return screenshot_grabber_->CanTakeScreenshot();
 }
 
 void ChromeScreenshotGrabber::PrepareFileAndRunOnBlockingPool(
     const base::FilePath& path,
     scoped_refptr<base::TaskRunner> blocking_task_runner,
     const FileCallback& callback) {
 #if defined(OS_CHROMEOS)
   Profile* profile = ProfileManager::GetActiveUserProfile();
@@ skipping 67 matching lines @@
 #endif
 
 void ChromeScreenshotGrabber::SetProfileForTest(Profile* profile) {
   profile_for_test_ = profile;
 }
 
 Profile* ChromeScreenshotGrabber::GetProfile() {
   return profile_for_test_ ? profile_for_test_
                            : ProfileManager::GetActiveUserProfile();
 }