Fix ExtensionServiceTest.ClearExtensionData flakiness

Fix ExtensionServiceTest.ClearExtensionData flakiness

The test runs work on the IO thread, but doesn't wait for the IO thread.
Fails ~17% of the time on ChromeOS

BUG=396504
R=ajwong@chromium.org
TBR=torne@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286674

[awong, 2014-07-28 15:05:25]

lgtm

nice fix!

https://codereview.chromium.org/421193002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/extension_service_unittest.cc (right):

https://codereview.chromium.org/421193002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/extension_service_unittest.cc:4339:
scoped_refptr<base::ThreadTestHelper> wait_for_io_thread(
unrelated: Why is ThreadTestHelper refcounted? :-/

[jochen (gone - plz use gerrit), 2014-07-28 15:09:27]

actually, that fix doesn't work :(

dom storage uses an internal worker pool that needs to be drained.

ThreadTestHelper is refcounted so it can posttask around internally. and it's
older than weak ptrs ...

[awong, 2014-07-28 15:11:28]

Is it possible to change the internal worker pool to use something like the
browser thread blocking pool and drain that?


On Mon, Jul 28, 2014 at 8:09 AM, <jochen@chromium.org> wrote:

> actually, that fix doesn't work :(
>
> dom storage uses an internal worker pool that needs to be drained.
>
> ThreadTestHelper is refcounted so it can posttask around internally. and
> it's
> older than weak ptrs ...
>
> https://codereview.chromium.org/421193002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2014-07-28 15:23:18]

StoragePartition::ClearData already takes a completion callback, I'm just
plumbing this through for ClearDataFromOrigin().... voila

PTAL

[jochen (gone - plz use gerrit), 2014-07-28 15:25:21]

The changes are:

- StoragePartition::ClearDataFromOrigin takes a callback argument
- plumbing:
  - c/b/e/data_deleter.cc pass the callback to storage partition
  - c/b/e/extension_service.cc pass the callback to data deleter
  - c/b/e/extension_service_unittests.cc pass a QuitClosure in
  - everywhere else, pass base::Closure() in

[awong, 2014-07-28 15:30:34]

LGTM

That's a lot of boilerplate but I like the explicitness.

[jochen (gone - plz use gerrit), 2014-07-29 08:35:35]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-07-29 08:37:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jochen@chromium.org/421193002/60001

[commit-bot: I haz the power, 2014-07-29 13:39:31]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-29 13:43:49]

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

[commit-bot: I haz the power, 2014-07-29 13:43:50]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[jochen (gone - plz use gerrit), 2014-07-30 13:52:31]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-07-30 13:53:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jochen@chromium.org/421193002/100001

[jochen (gone - plz use gerrit), 2014-07-30 15:20:54]

The CQ bit was unchecked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2014-07-30 15:21:33]

TBR'ing torne for adopting callsite in android_webview after api change

[jochen (gone - plz use gerrit), 2014-07-30 15:21:43]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-07-30 15:22:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jochen@chromium.org/421193002/100001

[Torne, 2014-07-30 15:37:04]

lgtm

[commit-bot: I haz the power, 2014-07-30 17:15:57]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-07-30 18:03:31]

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

[commit-bot: I haz the power, 2014-07-30 18:03:32]

Try jobs failed on following builders:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jochen (gone - plz use gerrit), 2014-07-30 20:49:19]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2014-07-30 20:59:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jochen@chromium.org/421193002/120001

[commit-bot: I haz the power, 2014-07-31 01:32:55]

Change committed as 286674