Revert of Force HostContentSettingsMap to be destroyed on its owning thread.

Revert of Force HostContentSettingsMap to be destroyed on its owning thread. (patchset #5 id:90001 of https://codereview.chromium.org/2581213002/ )

Reason for revert:
This might cause flakiness:
https://bugs.chromium.org/p/chromium/issues/detail?id=675722

Original issue's description:
> Force HostContentSettingsMap to be destroyed on its owning thread.
> 
> 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)
> 
> Committed: https://crrev.com/28fb80beda6b171647d037057543f77e5b78e170
> Cr-Commit-Position: refs/heads/master@{#439534}

TBR=vmpstr@chromium.org,erg@chromium.org,bauerb@chromium.org,dcheng@chromium.org,mukai@chromium.org,gab@chromium.org
BUG=674946, 665588, 622400, 675722

[yoichio, 2016-12-20 02:11:28]

Created Revert of Force HostContentSettingsMap to be destroyed on its owning
thread.

[yoichio, 2016-12-20 02:11:35]

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

[commit-bot: I haz the power, 2016-12-20 02:12:07]

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-12-20 02:12:09]

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

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

Dry run: This issue passed the CQ dry run.

[gab, 2016-12-20 16:54:34]

Description was changed from

==========
Revert of Force HostContentSettingsMap to be destroyed on its owning thread.
(patchset #5 id:90001 of https://codereview.chromium.org/2581213002/ )

Reason for revert:
This might cause flakiness:
https://bugs.chromium.org/p/chromium/issues/detail?id=675722

Original issue's description:
> Force HostContentSettingsMap to be destroyed on its owning thread.
> 
> 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)
> 
> Committed: https://crrev.com/28fb80beda6b171647d037057543f77e5b78e170
> Cr-Commit-Position: refs/heads/master@{#439534}

TBR=vmpstr@chromium.org,erg@chromium.org,bauerb@chromium.org,dcheng@chromium....
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=674946, 665588, 622400
==========

to

==========
Revert of Force HostContentSettingsMap to be destroyed on its owning thread.
(patchset #5 id:90001 of https://codereview.chromium.org/2581213002/ )

Reason for revert:
This might cause flakiness:
https://bugs.chromium.org/p/chromium/issues/detail?id=675722

Original issue's description:
> Force HostContentSettingsMap to be destroyed on its owning thread.
> 
> 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)
> 
> Committed: https://crrev.com/28fb80beda6b171647d037057543f77e5b78e170
> Cr-Commit-Position: refs/heads/master@{#439534}

TBR=vmpstr@chromium.org,erg@chromium.org,bauerb@chromium.org,dcheng@chromium....
BUG=674946, 665588, 622400, 675722
==========

[gab, 2016-12-20 16:55:12]

On 2016/12/20 02:11:28, yoichio wrote:
> Created Revert of Force HostContentSettingsMap to be destroyed on its owning
> thread.

Closing per http://crbug.com/675722 having been marked as WontFix (also, this
was a requirement for another CL that landed already so revert wouldn't work
as-is).