Fix for menus blocking user activity detection.

Fix for menus blocking user activity detection.

Menus used to block detecting user activity. The fix works by making the
UserActivityDetector a PlatformEventObserver rather than an
EventHandler.

BUG=462735
TEST=ui_base_unittests --gtest_filter=UserActivityDetectorTest.*

Committed: https://crrev.com/c655cc2a17c6c7fd113346bedc8433488c4b6dbe
Cr-Commit-Position: refs/heads/master@{#320400}

[afakhry, 2015-03-10 17:12:29]

afakhry@chromium.org changed reviewers:
+ oshima@chromium.org, sadrul@chromium.org

[afakhry, 2015-03-10 17:12:29]

@Oshima: Could you please review shell.cc?
@Sadrul: Could you please review user_activity_detector*.*?

Thanks!

[oshima, 2015-03-10 22:24:10]

https://codereview.chromium.org/998603003/diff/1/ash/shell.cc
File ash/shell.cc (left):

https://codereview.chromium.org/998603003/diff/1/ash/shell.cc#oldcode925
ash/shell.cc:925: AddPreTargetHandler(user_activity_detector_.get());
You need to remove the pre target handler in dtor.

[afakhry, 2015-03-10 22:28:59]

https://codereview.chromium.org/998603003/diff/1/ash/shell.cc
File ash/shell.cc (left):

https://codereview.chromium.org/998603003/diff/1/ash/shell.cc#oldcode925
ash/shell.cc:925: AddPreTargetHandler(user_activity_detector_.get());
On 2015/03/10 22:24:10, oshima wrote:
> You need to remove the pre target handler in dtor.

I did that already above, didn't I? Am I missing something?

[oshima, 2015-03-10 22:56:51]

https://codereview.chromium.org/998603003/diff/1/ash/shell.cc
File ash/shell.cc (left):

https://codereview.chromium.org/998603003/diff/1/ash/shell.cc#oldcode925
ash/shell.cc:925: AddPreTargetHandler(user_activity_detector_.get());
On 2015/03/10 22:28:58, afakhry wrote:
> On 2015/03/10 22:24:10, oshima wrote:
> > You need to remove the pre target handler in dtor.
> 
> I did that already above, didn't I? Am I missing something?

Oh, sorry. I somehow missed that change. ash lgtm

[sadrul, 2015-03-11 07:46:16]

LGTM

[afakhry, 2015-03-11 17:18:18]

The CQ bit was checked by afakhry@chromium.org

[commit-bot: I haz the power, 2015-03-11 17:19:13]

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

[afakhry, 2015-03-11 18:25:09]

The CQ bit was unchecked by afakhry@chromium.org

[afakhry, 2015-03-11 18:35:59]

afakhry@chromium.org changed reviewers:
+ jamescook@chromium.org

[afakhry, 2015-03-11 18:36:00]

For some reason my eclipse indexer missed showing me that it was also being
created in shell_desktop_controller_aura.cc.

@jamescook: Could you please review shell_desktop_controller_aura.cc? Thanks!

[James Cook, 2015-03-11 22:03:48]

lgtm

[afakhry, 2015-03-11 22:19:18]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2015-03-11 22:19:19]

The patchset sent to the CQ was uploaded after l-g-t-m from sadrul@chromium.org,
oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/998603003/#ps20001 (title:
"Removing the use of the UserActivityDetector as a pre-target handler in
shell_desktop_controller_au…")

[commit-bot: I haz the power, 2015-03-11 22:20:51]

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

[afakhry, 2015-03-11 22:48:47]

The CQ bit was unchecked by afakhry@chromium.org

[afakhry, 2015-03-11 22:54:59]

Removed a header from |user_activity_detector_unittest.cc| causing a compile
failure on mac.

[afakhry, 2015-03-12 01:05:14]

Found out that on platforms other than CrOs and Linux PlatformEventSource can be
null.

[afakhry, 2015-03-12 01:57:35]

@sadrul: Could you please take a look at the change I made to
|user_activity_detector_unittest.cc|?

After running the trybots I found out that
|PlatformEventSource::CreateDefault()| needed to initialize a
|PlatformEventSource| in the unit tests returns nullptr on platforms other than
CrOs and linux, causing the tests to crash on these platforms.

[afakhry, 2015-03-12 20:41:19]

@Sadrul: as agreed on IM, please review the changes I made to
|user_activity_detector.cc| and |user_activity_detector_unittest.cc|. Thanks!

[sadrul, 2015-03-12 20:44:12]

lgtm++

https://codereview.chromium.org/998603003/diff/80001/ui/base/user_activity/us...
File ui/base/user_activity/user_activity_detector_unittest.cc (right):

https://codereview.chromium.org/998603003/diff/80001/ui/base/user_activity/us...
ui/base/user_activity/user_activity_detector_unittest.cc:77:
scoped_ptr<TestPlatformEventSource> platform_event_source_;
You probably don't need scoped_ptr<>

[afakhry, 2015-03-12 20:47:32]

https://codereview.chromium.org/998603003/diff/80001/ui/base/user_activity/us...
File ui/base/user_activity/user_activity_detector_unittest.cc (right):

https://codereview.chromium.org/998603003/diff/80001/ui/base/user_activity/us...
ui/base/user_activity/user_activity_detector_unittest.cc:77:
scoped_ptr<TestPlatformEventSource> platform_event_source_;
On 2015/03/12 20:44:11, sadrul wrote:
> You probably don't need scoped_ptr<>

Just trying to be consistent with the existing code. Would you like me to change
it?

[afakhry, 2015-03-12 23:00:04]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2015-03-12 23:00:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, oshima@chromium.org, sadrul@chromium.org
Link to the patchset: https://codereview.chromium.org/998603003/#ps100001
(title: "Ash on Windows would fail because of that CHECK(), doing it only for
CrOS and Linux")

[commit-bot: I haz the power, 2015-03-12 23:00:29]

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

[commit-bot: I haz the power, 2015-03-12 23:04:08]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-03-12 23:04:43]

Patchset 6 (id:??) landed as
https://crrev.com/c655cc2a17c6c7fd113346bedc8433488c4b6dbe
Cr-Commit-Position: refs/heads/master@{#320400}

[Jun Mukai, 2015-03-13 01:47:38]

On 2015/03/12 23:04:43, I haz the power (commit-bot) wrote:
> Patchset 6 (id:??) landed as
> https://crrev.com/c655cc2a17c6c7fd113346bedc8433488c4b6dbe
> Cr-Commit-Position: refs/heads/master@{#320400}

Hello,
it seems ozone bot starts failing because of this.
See:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...
Sample failure:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%2...
> [9901:9901:0312/163244:466775474:FATAL:user_activity_detector.cc(56)] Check
failed: platform_event_source.

I am going to revert this CL. Sorry.

[Jun Mukai, 2015-03-13 01:48:17]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1006733002/ by mukai@chromium.org.

The reason for reverting is: Linux ChromiumOS Ozone bot fails.