Remove MessageLoop destruction observer from mojo::Watcher.

Remove MessageLoop destruction observer from mojo::Watcher.

mojo::Watcher currently registers itself as a destruction observer
of the MessageLoop on which it is instantiated. When it is notified
that the MessageLoop is being destroyed, it invokes its callback
with MOJO_RESULT_ABORTED.

This CL removes the MessageLoop destruction observer from
mojo::Watcher. This allows mojo::Watcher to be instantiated on a
thread that doesn't run a MessageLoop (e.g. TaskScheduler threads).
No existing code depends on the MOJO_RESULT_ABORTED notification.

BUG=604762
Committed: https://crrev.com/da6bd89a95fc8c444720f3d76ac67b91dfd70280
Cr-Commit-Position: refs/heads/master@{#430637}

[fdoray, 2016-11-04 20:44:59]

The CQ bit was checked by fdoray@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-04 20:45:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-04 22:44:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-04 22:44:28]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[fdoray, 2016-11-07 19:32:07]

Description was changed from

==========
Remove MOJO_RESULT_ABORTED.

BUG=
==========

to

==========
Remove unused MOJO_RESULT_ABORTED.

Motivation: This notification is sent from a MessageLoop destruction
observer. Many MessageLoop threads are being replaced with
base/task_scheduler which doesn't support destruction observers.

Why don't we support destruction observers in base/task_scheduler?
- mojo::Watcher is the only user of MessageLoop destruction observers
  on MessageLoop threads that we want to redirect to TaskScheduler.
- Nobody seems to depend on MOJO_RESULT_ABORTED  (no tests break
   and no memory leaks are detected when it is removed).

BUG=604762
==========

[fdoray, 2016-11-07 21:00:49]

Description was changed from

==========
Remove unused MOJO_RESULT_ABORTED.

Motivation: This notification is sent from a MessageLoop destruction
observer. Many MessageLoop threads are being replaced with
base/task_scheduler which doesn't support destruction observers.

Why don't we support destruction observers in base/task_scheduler?
- mojo::Watcher is the only user of MessageLoop destruction observers
  on MessageLoop threads that we want to redirect to TaskScheduler.
- Nobody seems to depend on MOJO_RESULT_ABORTED  (no tests break
   and no memory leaks are detected when it is removed).

BUG=604762
==========

to

==========
Remove unused MOJO_RESULT_ABORTED.

This MojoResult is no longer used now that InterfaceEndpointClient
observes MessageLoop destruction.

BUG=604762
==========

[fdoray, 2016-11-07 21:03:17]

fdoray@chromium.org changed reviewers:
+ yzshen@chromium.org

[fdoray, 2016-11-07 21:03:18]

PTAL

[yzshen1, 2016-11-07 21:37:18]

yzshen@chromium.org changed reviewers:
+ rockot@chromium.org

[yzshen1, 2016-11-07 21:37:19]

+Ken as reviewer

[yzshen1, 2016-11-07 21:40:19]

On 2016/11/07 21:37:19, yzshen1 wrote:
> +Ken as reviewer

Please note that mojo::Watcher is not just used by InterfaceEndpointClient. You
probably want to look at other use cases to ensure that their behavior won't
break without the observer, if you haven't done that.

[Ken Rockot(use gerrit already), 2016-11-07 21:50:01]

There are a few MojoResult values that are currently unused. I don't see much
value in removing their definitions yet, and I'd rather we refrain from doing
that at this time. Please don't do that in this CL.

Please also update the CL description to cover the actual interesting impact of
the CL, which is that it removes MessageLoop destruction observation from
mojo::Watcher.

[fdoray, 2016-11-08 14:07:26]

Description was changed from

==========
Remove unused MOJO_RESULT_ABORTED.

This MojoResult is no longer used now that InterfaceEndpointClient
observes MessageLoop destruction.

BUG=604762
==========

to

==========
Remove MessageLoop destruction observer from mojo::Watcher.

mojo::Watcher currently registers itself as a destruction observer
of the MessageLoop on which it is instantiated. When it is notified
that the MessageLoop is being destroyed, it invokes its callback
with MOJO_RESULT_ABORTED.

This CL removes the MessageLoop destruction observer from
mojo::Watcher. This allows mojo::Watcher to be instantiated on a
thread that doesn't run a MessageLoop (e.g. TaskScheduler threads).
No existing code depends on the MOJO_RESULT_ABORTED notification.

BUG=604762
==========

[fdoray, 2016-11-08 14:54:27]

PTAnL

On 2016/11/07 21:50:01, Ken Rockot wrote:
> There are a few MojoResult values that are currently unused. I don't see much
> value in removing their definitions yet, and I'd rather we refrain from doing
> that at this time. Please don't do that in this CL.
Done

> 
> Please also update the CL description to cover the actual interesting impact
of
> the CL, which is that it removes MessageLoop destruction observation from
> mojo::Watcher.
Done

I also audited all mojo::Watcher users and it doesn't seem like they depend on
the MOJO_RESULT_ABORTED notification.

[Ken Rockot(use gerrit already), 2016-11-08 15:13:22]

LGTM. Thanks!

[fdoray, 2016-11-08 15:20:04]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-11-08 15:20:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-08 16:57:48]

Description was changed from

==========
Remove MessageLoop destruction observer from mojo::Watcher.

mojo::Watcher currently registers itself as a destruction observer
of the MessageLoop on which it is instantiated. When it is notified
that the MessageLoop is being destroyed, it invokes its callback
with MOJO_RESULT_ABORTED.

This CL removes the MessageLoop destruction observer from
mojo::Watcher. This allows mojo::Watcher to be instantiated on a
thread that doesn't run a MessageLoop (e.g. TaskScheduler threads).
No existing code depends on the MOJO_RESULT_ABORTED notification.

BUG=604762
==========

to

==========
Remove MessageLoop destruction observer from mojo::Watcher.

mojo::Watcher currently registers itself as a destruction observer
of the MessageLoop on which it is instantiated. When it is notified
that the MessageLoop is being destroyed, it invokes its callback
with MOJO_RESULT_ABORTED.

This CL removes the MessageLoop destruction observer from
mojo::Watcher. This allows mojo::Watcher to be instantiated on a
thread that doesn't run a MessageLoop (e.g. TaskScheduler threads).
No existing code depends on the MOJO_RESULT_ABORTED notification.

BUG=604762
==========

[commit-bot: I haz the power, 2016-11-08 16:57:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-08 17:02:51]

Description was changed from

==========
Remove MessageLoop destruction observer from mojo::Watcher.

mojo::Watcher currently registers itself as a destruction observer
of the MessageLoop on which it is instantiated. When it is notified
that the MessageLoop is being destroyed, it invokes its callback
with MOJO_RESULT_ABORTED.

This CL removes the MessageLoop destruction observer from
mojo::Watcher. This allows mojo::Watcher to be instantiated on a
thread that doesn't run a MessageLoop (e.g. TaskScheduler threads).
No existing code depends on the MOJO_RESULT_ABORTED notification.

BUG=604762
==========

to

==========
Remove MessageLoop destruction observer from mojo::Watcher.

mojo::Watcher currently registers itself as a destruction observer
of the MessageLoop on which it is instantiated. When it is notified
that the MessageLoop is being destroyed, it invokes its callback
with MOJO_RESULT_ABORTED.

This CL removes the MessageLoop destruction observer from
mojo::Watcher. This allows mojo::Watcher to be instantiated on a
thread that doesn't run a MessageLoop (e.g. TaskScheduler threads).
No existing code depends on the MOJO_RESULT_ABORTED notification.

BUG=604762

Committed: https://crrev.com/da6bd89a95fc8c444720f3d76ac67b91dfd70280
Cr-Commit-Position: refs/heads/master@{#430637}
==========

[commit-bot: I haz the power, 2016-11-08 17:02:52]

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

[xlai (Olivia), 2016-11-08 19:04:02]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2480153004/ by xlai@chromium.org.

The reason for reverting is: I'm suspecting that this CL is causing these two
layout tests to crash on WebKit Linux Precise Leak bot:
broadcastchannel/blobs.html
broadcastchannel/workers.html

because they are crashing on mojo::SyncHandleRegistry in a check for watcher
pointer.

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Precis...

And this CL is the only CL in the build that touches on mojo watcher. 

.