Remove MessageLoop::current() from base_session_service.cc.

Remove MessageLoop::current() from base_session_service.cc.

Whenever possible, use ThreadTaskRunnerHandle::Get() instead of
MessageLoop::current(). ThreadTaskRunnerHandle::Get() works within
TaskScheduler while MessageLoop::current() doesn't.

Good reasons to use MessageLoop::current():
- Add destruction, nesting or task observers.
- Run nested loops.

Bad reasons to use MessageLoop::current():
- Post tasks. Use ThreadTaskRunnerHandle::Get() instead.
- Watch a file descriptor. Use FileDescriptorWatcher instead.
- Verify that it is possible to post tasks to the current thread.
  Use ThreadTaskRunnerHandle::IsSet() instead.
- Verify that code runs on a specific thread. Use
  SingleThreadTaskRunner::BelongsToCurrentThread() instead.

BUG=650723
Committed: https://crrev.com/8df1700462b359e414db9718d201cb3acb4e094e
Cr-Commit-Position: refs/heads/master@{#425363}

[fdoray, 2016-10-11 17:56:55]

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

[commit-bot: I haz the power, 2016-10-11 17:57:05]

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-10-11 19:24:52]

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

[commit-bot: I haz the power, 2016-10-11 19:24:53]

Dry run: This issue passed the CQ dry run.

[fdoray, 2016-10-11 19:41:37]

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

[fdoray, 2016-10-11 19:41:37]

PTAL

[Mr4D (OOO till 08-26), 2016-10-13 14:19:49]

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

[Mr4D (OOO till 08-26), 2016-10-13 14:19:49]

I think this is okay, but I defer to sky - as there might have been a reason.

[sky, 2016-10-13 17:55:02]

This code always run on the main thread.

[fdoray, 2016-10-13 17:57:05]

OK, can we still use ThreadTaskRunnerHandle::IsSet() instead of
MessageLoop::current()?

On Thu, Oct 13, 2016 at 1:55 PM <sky@chromium.org> wrote:

This code always run on the main thread.

https://codereview.chromium.org/2404353002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[fdoray, 2016-10-13 17:57:17]

On 2016/10/13 17:55:02, sky wrote:
> This code always run on the main thread.

OK, can we still use ThreadTaskRunnerHandle::IsSet() instead of
MessageLoop::current()?

[fdoray, 2016-10-13 17:59:02]

On 2016/10/13 17:57:17, fdoray wrote:
> On 2016/10/13 17:55:02, sky wrote:
> > This code always run on the main thread.
> 
> OK, can we still use ThreadTaskRunnerHandle::IsSet() instead of
> MessageLoop::current()?

It's easier for us to remove a MessageLoop::current() call site than to keep
track of which MessageLoop::current() call sites are guaranteed not to run on
threads that we want to migrate to TaskScheduler.

[sky, 2016-10-14 00:02:48]

Sorry I haven't kept up with all the name changes. Given this code
runs on the main thread could you clarify under what conditions the
two return different values?

Thanks,

  -Scott

On Thu, Oct 13, 2016 at 10:59 AM,  <fdoray@chromium.org> wrote:
> On 2016/10/13 17:57:17, fdoray wrote:
>> On 2016/10/13 17:55:02, sky wrote:
>> > This code always run on the main thread.
>>
>> OK, can we still use ThreadTaskRunnerHandle::IsSet() instead of
>> MessageLoop::current()?
>
> It's easier for us to remove a MessageLoop::current() call site than to keep
> track of which MessageLoop::current() call sites are guaranteed not to run
> on
> threads that we want to migrate to TaskScheduler.
>
> https://codereview.chromium.org/2404353002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[fdoray, 2016-10-14 12:02:10]

On 2016/10/14 00:02:48, sky wrote:
> Sorry I haven't kept up with all the name changes. Given this code
> runs on the main thread could you clarify under what conditions the
> two return different values?
> 
> Thanks,
> 
>   -Scott
> 
> On Thu, Oct 13, 2016 at 10:59 AM,  <mailto:fdoray@chromium.org> wrote:
> > On 2016/10/13 17:57:17, fdoray wrote:
> >> On 2016/10/13 17:55:02, sky wrote:
> >> > This code always run on the main thread.
> >>
> >> OK, can we still use ThreadTaskRunnerHandle::IsSet() instead of
> >> MessageLoop::current()?
> >
> > It's easier for us to remove a MessageLoop::current() call site than to keep
> > track of which MessageLoop::current() call sites are guaranteed not to run
> > on
> > threads that we want to migrate to TaskScheduler.
> >
> > https://codereview.chromium.org/2404353002/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
> 

MessageLoop::current() returns a non-nullptr MessageLoop from:
- any thread that runs a MessageLoop (including the main thread)
ThreadTaskRunnerHandle::IsSet() returns true from:
- any thread that runs a MessageLoop (including the main thread)
- a SINGLE_THREADED Task running inside TaskScheduler

In preparation for the migration of the DB, FILE, FILE_USER_BLOCKING,
PROCESS_LAUNCHER and CACHE threads to TaskScheduler, we want to replace most
MessageLoop::current() call sites with ThreadTaskRunnerHandle::Get/IsSet().
Since BaseSessionService::StartSaveTimer only runs on the main thread, this CL
isn't absolutely necessary. However, it's easier for us to migrate a
MessageLoop::current() call site to ThreadTaskRunnerHandle::Get/IsSet() than to
remember that it only runs on the main thread.

[sky, 2016-10-14 15:55:31]

Thanks for the clarification. I believe you're saying there is no difference in
this code, so, ok, LGTM

[fdoray, 2016-10-14 15:56:07]

On 2016/10/14 15:55:31, sky wrote:
> Thanks for the clarification. I believe you're saying there is no difference
in
> this code, so, ok, LGTM

Yes, there is no difference for this code.

[fdoray, 2016-10-14 15:56:14]

The CQ bit was checked by fdoray@chromium.org

[commit-bot: I haz the power, 2016-10-14 15:56:38]

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-10-14 16:56:43]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-14 16:59:34]

Description was changed from

==========
Remove MessageLoop::current() from base_session_service.cc.

Whenever possible, use ThreadTaskRunnerHandle::Get() instead of
MessageLoop::current(). ThreadTaskRunnerHandle::Get() works within
TaskScheduler while MessageLoop::current() doesn't.

Good reasons to use MessageLoop::current():
- Add destruction, nesting or task observers.
- Run nested loops.

Bad reasons to use MessageLoop::current():
- Post tasks. Use ThreadTaskRunnerHandle::Get() instead.
- Watch a file descriptor. Use FileDescriptorWatcher instead.
- Verify that it is possible to post tasks to the current thread.
  Use ThreadTaskRunnerHandle::IsSet() instead.
- Verify that code runs on a specific thread. Use
  SingleThreadTaskRunner::BelongsToCurrentThread() instead.

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

to

==========
Remove MessageLoop::current() from base_session_service.cc.

Whenever possible, use ThreadTaskRunnerHandle::Get() instead of
MessageLoop::current(). ThreadTaskRunnerHandle::Get() works within
TaskScheduler while MessageLoop::current() doesn't.

Good reasons to use MessageLoop::current():
- Add destruction, nesting or task observers.
- Run nested loops.

Bad reasons to use MessageLoop::current():
- Post tasks. Use ThreadTaskRunnerHandle::Get() instead.
- Watch a file descriptor. Use FileDescriptorWatcher instead.
- Verify that it is possible to post tasks to the current thread.
  Use ThreadTaskRunnerHandle::IsSet() instead.
- Verify that code runs on a specific thread. Use
  SingleThreadTaskRunner::BelongsToCurrentThread() instead.

BUG=650723

Committed: https://crrev.com/8df1700462b359e414db9718d201cb3acb4e094e
Cr-Commit-Position: refs/heads/master@{#425363}
==========

[commit-bot: I haz the power, 2016-10-14 16:59:35]

Patchset 1 (id:??) landed as
https://crrev.com/8df1700462b359e414db9718d201cb3acb4e094e
Cr-Commit-Position: refs/heads/master@{#425363}