Remove ModelSafeWorker::work_done_or_stopped().

Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=650723
Committed: https://crrev.com/119462f5be2cba878cf3d30142411a06868ed464
Cr-Commit-Position: refs/heads/master@{#429994}

[fdoray, 2016-11-01 19:47:14]

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

[fdoray, 2016-11-01 19:47:14]

PTAL

[fdoray, 2016-11-01 19:48:25]

fdoray@chromium.org changed reviewers:
+ zea@chromium.org
- sky@chromium.org

[fdoray, 2016-11-01 19:48:25]

-sky@
+zea@: PTAL

[fdoray, 2016-11-01 19:53:54]

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

[commit-bot: I haz the power, 2016-11-01 19:54:17]

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

[fdoray, 2016-11-01 20:12:07]

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

[commit-bot: I haz the power, 2016-11-01 20:12:35]

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-01 21:39:34]

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

[commit-bot: I haz the power, 2016-11-01 21:39:34]

Dry run: This issue passed the CQ dry run.

[Nicolas Zea, 2016-11-01 22:39:24]

Mostly LG, some nits and a unit tests request.

https://codereview.chromium.org/2466313003/diff/60001/components/history/core...
File components/history/core/browser/history_model_worker.cc (right):

https://codereview.chromium.org/2466313003/diff/60001/components/history/core...
components/history/core/browser/history_model_worker.cc:122: base::WaitableEvent
work_done_or_abandonned(
spelling nit: s/abandonned/abandoned (here and in other files)

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
File components/sync/engine/signal_event_on_delete.cc (right):

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
components/sync/engine/signal_event_on_delete.cc:12:
SignalEventOnDelete::SignalEventOnDelete(base::WaitableEvent* event)
Although this is all pretty basic logic, would be good to have a few simple unit
tests (which are also a useful reference for how to use this kind of object).

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
File components/sync/engine/signal_event_on_delete.h (right):

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
components/sync/engine/signal_event_on_delete.h:5: #ifndef
COMPONENTS_SYNC_ENGINE_SIGNAL_EVENT_ON_DELETE_H_
I think it would be cleaner to define this file in components/sync/base than in
the engine.

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
components/sync/engine/signal_event_on_delete.h:18: class SignalEventOnDelete {
I wonder if it would make more sense to rename this ScopedEventSignal or
something like that (following the Scoped* naming pattern)

[fdoray, 2016-11-02 15:52:05]

zea@: PTAnL

https://codereview.chromium.org/2466313003/diff/60001/components/history/core...
File components/history/core/browser/history_model_worker.cc (right):

https://codereview.chromium.org/2466313003/diff/60001/components/history/core...
components/history/core/browser/history_model_worker.cc:122: base::WaitableEvent
work_done_or_abandonned(
On 2016/11/01 22:39:23, Nicolas Zea wrote:
> spelling nit: s/abandonned/abandoned (here and in other files)

Done.

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
File components/sync/engine/signal_event_on_delete.cc (right):

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
components/sync/engine/signal_event_on_delete.cc:12:
SignalEventOnDelete::SignalEventOnDelete(base::WaitableEvent* event)
On 2016/11/01 22:39:23, Nicolas Zea wrote:
> Although this is all pretty basic logic, would be good to have a few simple
unit
> tests (which are also a useful reference for how to use this kind of object). 

Done.

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
File components/sync/engine/signal_event_on_delete.h (right):

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
components/sync/engine/signal_event_on_delete.h:5: #ifndef
COMPONENTS_SYNC_ENGINE_SIGNAL_EVENT_ON_DELETE_H_
On 2016/11/01 22:39:23, Nicolas Zea wrote:
> I think it would be cleaner to define this file in components/sync/base than
in
> the engine.

Done.

https://codereview.chromium.org/2466313003/diff/60001/components/sync/engine/...
components/sync/engine/signal_event_on_delete.h:18: class SignalEventOnDelete {
On 2016/11/01 22:39:23, Nicolas Zea wrote:
> I wonder if it would make more sense to rename this ScopedEventSignal or
> something like that (following the Scoped* naming pattern)

Done.

[Nicolas Zea, 2016-11-02 20:35:05]

LGTM, thanks!

[fdoray, 2016-11-02 20:47:17]

fdoray@chromium.org changed reviewers:
+ melandory@chromium.org, sky@chromium.org

[fdoray, 2016-11-02 20:47:18]

sky@: Please review changes in components/history
melandory@: Please review changes in components/password_manager

[sky, 2016-11-02 21:28:13]

LGTM

[fdoray, 2016-11-04 12:29:41]

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

[fdoray, 2016-11-04 12:29:41]

+isherman@

melandory@ or isherman@: PTAL at components/password_manager.

[Ilya Sherman, 2016-11-04 17:25:42]

password_manager lgtm, thanks

[fdoray, 2016-11-04 17:27:31]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-11-04 17:28:00]

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

[fdoray, 2016-11-04 18:05:01]

Description was changed from

==========
Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=653916
==========

to

==========
Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=650723
==========

[commit-bot: I haz the power, 2016-11-04 20:17:35]

Description was changed from

==========
Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=650723
==========

to

==========
Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=650723
==========

[commit-bot: I haz the power, 2016-11-04 20:17:41]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-04 20:19:18]

Description was changed from

==========
Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=650723
==========

to

==========
Remove ModelSafeWorker::work_done_or_stopped().

BrowserThreads will be migrated to TaskScheduler which doesn't support
destruction observers. This CL removes an event signaled after a task
runs or from ModelSafeWorker::WillDestroyCurrentMessageLoop() with an
event signaled after a task runs or after it is deleted (MessageLoop
deletes pending tasks in its destructor).

BUG=650723

Committed: https://crrev.com/119462f5be2cba878cf3d30142411a06868ed464
Cr-Commit-Position: refs/heads/master@{#429994}
==========

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

Patchset 5 (id:??) landed as
https://crrev.com/119462f5be2cba878cf3d30142411a06868ed464
Cr-Commit-Position: refs/heads/master@{#429994}