EndSessionWatcherWindow implementation.

EndSessionWatcherWindow implementation.

BUG=412384
Committed: https://crrev.com/24afa73de8b4c7a89ebf8fd60c1fc9df55ef5aaf
Cr-Commit-Position: refs/heads/master@{#311361}

[Sigurður Ásgeirsson, 2015-01-09 18:28:49]

siggi@chromium.org changed reviewers:
+ erikwright@chromium.org

[Sigurður Ásgeirsson, 2015-01-09 18:28:50]

PTAL, though I've yet to verify that this is useful where I intend to use it (so
no rush).

[erikwright (departed), 2015-01-13 21:01:29]

LGTM with nits.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
File components/browser_watcher/endsession_watcher_window_win.cc (right):

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:4: #include
"components/browser_watcher/endsession_watcher_window_win.h"
blank line before

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:19: WNDCLASSEX
window_class;
 = {0}

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:32:
::SetWindowLongPtr(window_, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(this));
Is it crazy to check for an error?

Apparently it would involve a ::SetLastError(0) before the call, and a
DCHECK_EQ(0, ::GetLastError()) after.

http://msdn.microsoft.com/en-us/library/windows/desktop/ms644898%28v=vs.85%29...

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:73: 
only one blank

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
File components/browser_watcher/endsession_watcher_window_win.h (right):

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.h:4: #ifndef
COMPONENTS_BROWSER_WATCHER_ENDSESSION_WATCHER_WINDOW_WIN_H_
blank line before

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.h:17: explicit
EndSessionWatcherWindow(EndSessionCallback on_end_session);
const&

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.h:38:
DISALLOW_COPY_AND_ASSIGN(EndSessionWatcherWindow);
macros.h

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
File components/browser_watcher/endsession_watcher_window_win_unittest.cc
(right):

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win_unittest.cc:4: #include
"components/browser_watcher/endsession_watcher_window_win.h"
blank line before

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win_unittest.cc:17: void
OnEndSession(bool* called, LPARAM* out_lparam, LPARAM lparam) {
this could be a free function, and you could get rid of the fixture.

[Sigurður Ásgeirsson, 2015-01-13 22:21:42]

Patchset #3 (id:40001) has been deleted

[Sigurður Ásgeirsson, 2015-01-13 22:41:49]

The CQ bit was checked by siggi@chromium.org

[Sigurður Ásgeirsson, 2015-01-13 22:41:57]

Thanks, committing.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
File components/browser_watcher/endsession_watcher_window_win.cc (right):

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:4: #include
"components/browser_watcher/endsession_watcher_window_win.h"
On 2015/01/13 21:01:28, erikwright wrote:
> blank line before

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:19: WNDCLASSEX
window_class;
On 2015/01/13 21:01:28, erikwright wrote:
>  = {0}

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:32:
::SetWindowLongPtr(window_, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(this));
On 2015/01/13 21:01:28, erikwright wrote:
> Is it crazy to check for an error?
> 
> Apparently it would involve a ::SetLastError(0) before the call, and a
> DCHECK_EQ(0, ::GetLastError()) after.
> 
>
http://msdn.microsoft.com/en-us/library/windows/desktop/ms644898%28v=vs.85%29...

Not crazy - maybe a little ... deranged :).
Added a check for the expected value after set - how's that?

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.cc:73: 
On 2015/01/13 21:01:28, erikwright wrote:
> only one blank

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
File components/browser_watcher/endsession_watcher_window_win.h (right):

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.h:4: #ifndef
COMPONENTS_BROWSER_WATCHER_ENDSESSION_WATCHER_WINDOW_WIN_H_
On 2015/01/13 21:01:28, erikwright wrote:
> blank line before

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.h:17: explicit
EndSessionWatcherWindow(EndSessionCallback on_end_session);
On 2015/01/13 21:01:28, erikwright wrote:
> const&

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win.h:38:
DISALLOW_COPY_AND_ASSIGN(EndSessionWatcherWindow);
On 2015/01/13 21:01:29, erikwright wrote:
> macros.h

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
File components/browser_watcher/endsession_watcher_window_win_unittest.cc
(right):

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win_unittest.cc:4: #include
"components/browser_watcher/endsession_watcher_window_win.h"
On 2015/01/13 21:01:29, erikwright wrote:
> blank line before

Done.

https://codereview.chromium.org/843113002/diff/20001/components/browser_watch...
components/browser_watcher/endsession_watcher_window_win_unittest.cc:17: void
OnEndSession(bool* called, LPARAM* out_lparam, LPARAM lparam) {
On 2015/01/13 21:01:29, erikwright wrote:
> this could be a free function, and you could get rid of the fixture.

So true, so much less verbose.

[commit-bot: I haz the power, 2015-01-13 22:42:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/843113002/80001

[commit-bot: I haz the power, 2015-01-13 23:35:01]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-01-13 23:36:40]

Patchset 4 (id:??) landed as
https://crrev.com/24afa73de8b4c7a89ebf8fd60c1fc9df55ef5aaf
Cr-Commit-Position: refs/heads/master@{#311361}