GitLab

It uses WeakPtrFactory and as such must be destroyed on its owning thread (UI).

The rest of the changes can then be inferred from the stack trace on the bug:

1) RefcountedKeyedServiceTraits::Destruct() must check for
   ThreadTaskRunnerHandle::IsSet() before invoking TTRH::Get().
  ** UPDATE in PS 5 : Use STR::RunsTasksOnCurrentThread() instead of STRH comparison.

2) Nothing about RefcountedKeyedService's use of its |task_runner| is thread-affine
   => using SequencedTaskRunner instead is inline with our desire to make all
   top-level API use sequence-affinity instead of thread-affinity for thread-safety.

3) Calling SequencedTaskRunnerHandle::IsSet() from the Callback's destructor requires
   the SequencedWorkerPool's task info to be set in that scope
   => brought changes from same requirements in another CL
      @ https://codereview.chromium.org/2491613004/#ps160001
   Note: intentionally not adding tests as this change highlights that this deletion is
   racy in theory (deleting without owning the sequence means potentially deleting
   things from the same SequenceToken in parallel... any good test would fail TSAN and
   I also don't want to address this issue given it's long standing and SWP is being
   replaced by TaskScheduler shortly).

4) Switch to ScopedMockTimeMessageLoopTaskRunner in NotificationPermissionContextTest
   => otherwise LSAN catches a leak in patch set 3 as the HostContentSettingsMap is
      sent for deletion on the wrong task runner and the task is never flushed.
      (ScopedMockTimeMessageLoopTaskRunner ensures the loop is flushed before replacing
       it, both ways).

Precursor requirement to https://codereview.chromium.org/2576243003/ which
for some reason makes this destruction race come to life.

BUG=674946, 665588, 622400
TBR=mukai (NotificationPermissionContextTest side-effects)

Review-Url: https://codereview.chromium.org/2581213002
Cr-Commit-Position: refs/heads/master@{#439534}