Migrate Extensions code to Task Scheduler API

Migrate Extensions Store/Policy code to Task Scheduler API

This changes most of Extensions Store/Policy code to use a dedicated
SequencedTaskRunner to post tasks rather than posting on the browser
FILE thread.

Initially I tried to keep the change limited to the sync related
part of the Extensions code. The problem that I ran into is that
different types of storage are handled in a generic way on the
storage frontend (for example, all them were deleted on FILE thread).
So once I migrated the sync storage, I had to modify other storage
types as well. Because of that I had to access backend task
runner singleton from extensions component so in order to satisfy
dependencies I had to move GetBackendTaskRunner() to 
extensions/browser/api/storage.

Some of the tests relied on TestBrowserThreadBundle making all
browser threads map to the same physical thread which allowed the
test code to satisfy DCHECK_CURRENTLY_ON(BrowserThread::FILE) check
while running on the main test thread. With this migration that
didn't work anymore so I had to modify ExtensionSettingSyncTest and
PolicyValueStoreTest tests to actually post API calls on the right
sequence. That was the most difficult part of this change.
Other than that the rest of the changes should be straightforward.

BUG=689520
Review-Url: https://codereview.chromium.org/2965153002
Cr-Commit-Position: refs/heads/master@{#486250}
Committed: https://chromium.googlesource.com/chromium/src/+/6d016c9355721f174c345002f89cb9f750655f6a

[stanisc, 2017-07-06 00:34:47]

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

[commit-bot: I haz the power, 2017-07-06 00:35: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, 2017-07-06 01:18:59]

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

[commit-bot: I haz the power, 2017-07-06 01:19:00]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[stanisc, 2017-07-11 22:25:01]

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

[commit-bot: I haz the power, 2017-07-11 22:25:40]

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, 2017-07-11 23:29:21]

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

[commit-bot: I haz the power, 2017-07-11 23:29:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[stanisc, 2017-07-12 01:38:16]

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

[commit-bot: I haz the power, 2017-07-12 01:38:46]

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, 2017-07-12 03:19:50]

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

[commit-bot: I haz the power, 2017-07-12 03:19:51]

Dry run: This issue passed the CQ dry run.

[stanisc, 2017-07-12 18:12:32]

Description was changed from

==========
Migrated extensions code

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

to

==========
Migrate Extensions Store/Policy code to Task Scheduler API

This changes most of Extensions Store/Policy code to use a dedicated
SequencedTaskRunner to post tasks rather than posting on the browser
FILE thread.

Initially I tried to keep the change limited to the sync related
part of the Extensions code. The problem that I ran into is that
different types of storage are handled in a generic way on the
storage frontend (for example, all them were deleted on FILE thread).
So once I migrated the sync storage, I had to modify other storage
types as well. Because of that I had to access backend task
runner singleton from extensions component so in order to satisfy
dependencies I had to move GetBackendTaskRunner() to 
extensions/browser/api/storage.

Some of the tests relied on TestBrowserThreadBundle making all
browser threads map to the same physical thread which allowed the
test code to satisfy DCHECK_CURRENTLY_ON(BrowserThread::FILE) check
while running on the main test thread. With this migration that
didn't work anymore so I had to modify ExtensionSettingSyncTest and
PolicyValueStoreTest tests to actually post API calls on the right
sequence. That was the most difficult part of this change.
Other than that the rest of the changes should be straightforward.

BUG=689520
==========

[stanisc, 2017-07-12 18:28:11]

stanisc@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[stanisc, 2017-07-12 18:28:12]

PTAL!

[Devlin, 2017-07-12 22:58:00]

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/storage/settings_sync_unittest.cc (left):

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/storage/settings_sync_unittest.cc:422: // Maintain
dictionaries mirrored to the expected values of the settings in
Reitveld's struggling with some of these diffs - are these all copy-paste-git cl
format?

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/storage/settings_sync_unittest.cc (right):

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/storage/settings_sync_unittest.cc:259: void
PostOnBackendSequenceAndWait(const tracked_objects::Location& from_here,
Heh, clever. :)

[stanisc, 2017-07-12 23:42:49]

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/storage/settings_sync_unittest.cc (left):

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/storage/settings_sync_unittest.cc:422: // Maintain
dictionaries mirrored to the expected values of the settings in
On 2017/07/12 22:58:00, Devlin wrote:
> Reitveld's struggling with some of these diffs - are these all copy-paste-git
cl
> format?

Yes, more or less. Some of code blocks had to be moved to be inside or outside
of PostOnBackendSequenceAndWait scope via copy-paste. Other than that and  and
indenting the code, and running 'git cl format' I didn't do any changes.

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
File chrome/browser/extensions/api/storage/settings_sync_unittest.cc (right):

https://codereview.chromium.org/2965153002/diff/80001/chrome/browser/extensio...
chrome/browser/extensions/api/storage/settings_sync_unittest.cc:259: void
PostOnBackendSequenceAndWait(const tracked_objects::Location& from_here,
On 2017/07/12 22:58:00, Devlin wrote:
> Heh, clever. :)

After failing to use lambda directly with base::Bind I stole this idea from
LambdaBinder in sync_method_unittest.cc.

[Devlin, 2017-07-13 00:54:18]

lgtm; thanks for doing this!

[stanisc, 2017-07-13 01:25:04]

The CQ bit was checked by stanisc@chromium.org

[commit-bot: I haz the power, 2017-07-13 01:25:31]

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, 2017-07-13 03:12:24]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499909104111710,
"parent_rev": "65f31455665752ce794616118764379eaf49c6aa", "commit_rev":
"6d016c9355721f174c345002f89cb9f750655f6a"}

[commit-bot: I haz the power, 2017-07-13 03:12:39]

Description was changed from

==========
Migrate Extensions Store/Policy code to Task Scheduler API

This changes most of Extensions Store/Policy code to use a dedicated
SequencedTaskRunner to post tasks rather than posting on the browser
FILE thread.

Initially I tried to keep the change limited to the sync related
part of the Extensions code. The problem that I ran into is that
different types of storage are handled in a generic way on the
storage frontend (for example, all them were deleted on FILE thread).
So once I migrated the sync storage, I had to modify other storage
types as well. Because of that I had to access backend task
runner singleton from extensions component so in order to satisfy
dependencies I had to move GetBackendTaskRunner() to 
extensions/browser/api/storage.

Some of the tests relied on TestBrowserThreadBundle making all
browser threads map to the same physical thread which allowed the
test code to satisfy DCHECK_CURRENTLY_ON(BrowserThread::FILE) check
while running on the main test thread. With this migration that
didn't work anymore so I had to modify ExtensionSettingSyncTest and
PolicyValueStoreTest tests to actually post API calls on the right
sequence. That was the most difficult part of this change.
Other than that the rest of the changes should be straightforward.

BUG=689520
==========

to

==========
Migrate Extensions Store/Policy code to Task Scheduler API

This changes most of Extensions Store/Policy code to use a dedicated
SequencedTaskRunner to post tasks rather than posting on the browser
FILE thread.

Initially I tried to keep the change limited to the sync related
part of the Extensions code. The problem that I ran into is that
different types of storage are handled in a generic way on the
storage frontend (for example, all them were deleted on FILE thread).
So once I migrated the sync storage, I had to modify other storage
types as well. Because of that I had to access backend task
runner singleton from extensions component so in order to satisfy
dependencies I had to move GetBackendTaskRunner() to 
extensions/browser/api/storage.

Some of the tests relied on TestBrowserThreadBundle making all
browser threads map to the same physical thread which allowed the
test code to satisfy DCHECK_CURRENTLY_ON(BrowserThread::FILE) check
while running on the main test thread. With this migration that
didn't work anymore so I had to modify ExtensionSettingSyncTest and
PolicyValueStoreTest tests to actually post API calls on the right
sequence. That was the most difficult part of this change.
Other than that the rest of the changes should be straightforward.

BUG=689520

Review-Url: https://codereview.chromium.org/2965153002
Cr-Commit-Position: refs/heads/master@{#486250}
Committed:
https://chromium.googlesource.com/chromium/src/+/6d016c9355721f174c345002f89c...
==========

[commit-bot: I haz the power, 2017-07-13 03:12:41]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/6d016c9355721f174c345002f89c...