Remove window_snapshot.*

Remove window_snapshot.*

The two simple functions here only have one caller each, so move them
closer to where they are used.

BUG=none

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

[enne (OOO), 2014-01-09 19:36:22]

mukai: review
ben: chrome/browser/ui OWNERS
pam: chrome/browser/prefs OWNERS

Depends on https://codereview.chromium.org/126373002/ and
https://codereview.chromium.org/124393006/ to remove other callers of
GrabWindowSnapshotForUser.

[Jun Mukai, 2014-01-09 19:44:32]

lgtm, I like this.
It might be good to ask Mac guys (like thakis) for reviewing cocoa code just in
case.

[Pam (message me for reviews), 2014-01-09 19:44:33]

prefs lgtm

- Pam

[enne (OOO), 2014-01-09 19:45:41]

+thakis FYI for chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm if
you'd like to review

[Nico, 2014-01-09 19:50:39]

lgtm, but the panels code is dimich's (so cc'ing him fhi)

[jianli, 2014-01-10 00:47:36]

https://codereview.chromium.org/132203004/diff/1/chrome/browser/ui/cocoa/pane...
File chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm (right):

https://codereview.chromium.org/132203004/diff/1/chrome/browser/ui/cocoa/pane...
chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm:308: if
(ui::GrabWindowSnapshot(panel->GetNativeWindow(), &png, snapshot_bounds))
Should we break out for loop when the function returns false?

[enne (OOO), 2014-01-10 01:29:38]

https://codereview.chromium.org/132203004/diff/1/chrome/browser/ui/cocoa/pane...
File chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm (right):

https://codereview.chromium.org/132203004/diff/1/chrome/browser/ui/cocoa/pane...
chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm:308: if
(ui::GrabWindowSnapshot(panel->GetNativeWindow(), &png, snapshot_bounds))
On 2014/01/10 00:47:37, jianli wrote:
> Should we break out for loop when the function returns false?

I don't know this code very well.  Breaking out of the loop preserves the
previous behavior.

[jianli, 2014-01-10 07:12:09]

On Thu, Jan 9, 2014 at 5:29 PM, <enne@chromium.org> wrote:

>
> https://codereview.chromium.org/132203004/diff/1/chrome/
> browser/ui/cocoa/panels/panel_stack_window_cocoa.mm
> File chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm (right):
>
> https://codereview.chromium.org/132203004/diff/1/chrome/
> browser/ui/cocoa/panels/panel_stack_window_cocoa.mm#newcode308
> chrome/browser/ui/cocoa/panels/panel_stack_window_cocoa.mm:308: if
> (ui::GrabWindowSnapshot(panel->GetNativeWindow(), &png,
> snapshot_bounds))
> On 2014/01/10 00:47:37, jianli wrote:
>
>> Should we break out for loop when the function returns false?
>>
>
> I don't know this code very well.  Breaking out of the loop preserves
> the previous behavior.
>

We used to break out of the loop when chrome::GrabWindowSnapshotForUser
returns false:
 if (!chrome::GrabWindowSnapshotForUser(...))

It is changed to:
 if (ui::GrabWindowSnapshot(...))

Do we miss "!" in the condition?


> https://codereview.chromium.org/132203004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[enne (OOO), 2014-01-16 20:54:18]

On 2014/01/10 07:12:09, jianli wrote:

> We used to break out of the loop when chrome::GrabWindowSnapshotForUser
> returns false:
>  if (!chrome::GrabWindowSnapshotForUser(...))
> 
> It is changed to:
>  if (ui::GrabWindowSnapshot(...))
> 
> Do we miss "!" in the condition?

Oh!  Quite right.  Sorry for misunderstanding; thanks for pointing that out.  :)

[jianli, 2014-01-17 00:46:19]

Panels lgtm

[commit-bot: I haz the power, 2014-01-17 00:47:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/132203004/240001

[commit-bot: I haz the power, 2014-01-17 00:48:01]

Failed to apply patch for chrome/chrome_browser_ui.gypi:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/chrome_browser_ui.gypi
  Hunk #1 FAILED at 2621.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/chrome_browser_ui.gypi.rej

Patch:       chrome/chrome_browser_ui.gypi
Index: chrome/chrome_browser_ui.gypi
diff --git a/chrome/chrome_browser_ui.gypi b/chrome/chrome_browser_ui.gypi
index
4828ab68bfcca1aa68bc6778776fbf8986e85397..e1d98d6b08fb25b734c4637edf4b1975e3366558
100644
--- a/chrome/chrome_browser_ui.gypi
+++ b/chrome/chrome_browser_ui.gypi
@@ -2621,8 +2621,6 @@
         'browser/ui/window_sizer/window_sizer_gtk.cc',
         'browser/ui/window_sizer/window_sizer_mac.mm',
         'browser/ui/window_sizer/window_sizer_win.cc',
-        'browser/ui/window_snapshot/window_snapshot.cc',
-        'browser/ui/window_snapshot/window_snapshot.h',
         'browser/ui/zoom/zoom_controller.cc',
         'browser/ui/zoom/zoom_controller.h',
         'browser/ui/zoom/zoom_observer.h',

[commit-bot: I haz the power, 2014-01-17 00:52:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/132203004/240002

[commit-bot: I haz the power, 2014-01-17 01:25:31]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2014-01-17 02:36:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/132203004/550001

[commit-bot: I haz the power, 2014-01-17 06:42:45]

Change committed as 245444