Provide the custom thumbnail for live preview of stacked panels

We now changed to always provide the custom thumbnail for stacked panels.

The following issues have been fixed:
1) If one or more panels in the stack are collapsed, the live preview is not shown correctly.
2) If the whole stack is minimized by the system, the live preview is not shown correctly.

BUG=177743
TEST=Manual tests by collapsing a panel or system-minimzing a stack and then hover over the taskbar icon

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203016

[Dmitry Titov, 2013-05-29 01:14:48]

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
File chrome/browser/ui/views/panels/panel_stack_view.cc (right):

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/panel_stack_view.cc:489: void
PanelStackView::StartOrStopCustomThumbnailForMinimizePanel() {
Is it possible to have a StartThumbnailer() and StopThumbnailer() separately?
It could be more readable then create a single method that combines all
behaviors so it can be used from 3 places...

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
File chrome/browser/ui/views/panels/panel_stack_view.h (right):

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/panel_stack_view.h:85: #if defined(OS_WIN)
Could you please group all OS_WIN things under a single #ifdef?

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/panel_stack_view.h:117: void
CaptureThumbnailForLivePreview(bool should_capture_now);
This should be named like "StartThumbnailer", because it is what it does.

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
File chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h (right):

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h:30: // If
|capture_now| is true, the snapshot image is captured immediately.
capture_now -> should_capture_now
Can we rename it to "capture_first_snapshot_immediately"?

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h:52:
TaskbarWindowThumbnailerDelegateWin* delegate_;
Please add a comment on Weak or not, who owns, why this pointer is always valid
during lifetime of the thumbnailer.

[jianli, 2013-05-29 18:31:52]

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
File chrome/browser/ui/views/panels/panel_stack_view.cc (right):

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/panel_stack_view.cc:489: void
PanelStackView::StartOrStopCustomThumbnailForMinimizePanel() {
On 2013/05/29 01:14:48, Dmitry Titov wrote:
> Is it possible to have a StartThumbnailer() and StopThumbnailer() separately?
> It could be more readable then create a single method that combines all
> behaviors so it can be used from 3 places...

All 3 places need the same code because otherwise we will have the following
simplified duplicated code in all 3 places and we need to put comments too.
  StopThumbnailerForLivePreview();
  if (HasMinimizedPanels())
    StartThumbnailerForLivePreview(false);

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
File chrome/browser/ui/views/panels/panel_stack_view.h (right):

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/panel_stack_view.h:85: #if defined(OS_WIN)
On 2013/05/29 01:14:48, Dmitry Titov wrote:
> Could you please group all OS_WIN things under a single #ifdef?

Done.

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
File chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h (right):

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h:30: // If
|capture_now| is true, the snapshot image is captured immediately.
On 2013/05/29 01:14:48, Dmitry Titov wrote:
> capture_now -> should_capture_now
> Can we rename it to "capture_first_snapshot_immediately"?

Renamed to capture_snapshot_immediately.

https://codereview.chromium.org/16035007/diff/1/chrome/browser/ui/views/panel...
chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h:52:
TaskbarWindowThumbnailerDelegateWin* delegate_;
On 2013/05/29 01:14:48, Dmitry Titov wrote:
> Please add a comment on Weak or not, who owns, why this pointer is always
valid
> during lifetime of the thumbnailer.

Done.

[jianli, 2013-05-29 20:53:04]

Changed to always use the custom snapshot for stack window, as discussed.

[Dmitry Titov, 2013-05-29 21:12:54]

lgtm with 2 nits:

https://codereview.chromium.org/16035007/diff/39001/chrome/browser/ui/views/p...
File chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h (right):

https://codereview.chromium.org/16035007/diff/39001/chrome/browser/ui/views/p...
chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h:16: // Returns
the handle list of all windows that are used to construct the
"handle list of all windows" -> "list of handles for all windows"?

https://codereview.chromium.org/16035007/diff/39001/chrome/browser/ui/views/p...
chrome/browser/ui/views/panels/taskbar_window_thumbnailer_win.h:31: // captured
once when the system requests it.
Lets add "... when system requests it, so the updates of the panels' content
will not be automatically reflected in the thumbnail."

[commit-bot: I haz the power, 2013-05-29 21:19:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jianli@chromium.org/16035007/51001

[commit-bot: I haz the power, 2013-05-30 00:11:18]

Change committed as 203016