Componentize bookmark_expanded_state_tracker_unittest.cc.

Componentize bookmark_expanded_state_tracker_unittest.cc.

BUG=367831
TEST=components_unittests --gtest_filter=BookmarkExpandedStateTrackerTest.*
R=sky@chromium.org,blundell@chromium.org
NOTRY=true

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

[blundell, 2014-05-09 07:42:53]

not sure if this will fix your problems, but they're the right changes to make
in any case.

https://codereview.chromium.org/275773002/diff/20001/components/bookmarks/cor...
File
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc
(right):

https://codereview.chromium.org/275773002/diff/20001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:39:
scoped_ptr<base::MessageLoop> message_loop_;
This can just be a base::MessageLoop.

https://codereview.chromium.org/275773002/diff/20001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:63:
ui_task_runner_);
Don't create a separate ui_task_runner_; pass MessageLoopProxy::current().

[tfarina, 2014-05-09 16:53:53]

Colin, thanks a lot for your review!

With your suggestions + RunLoop at TearDown I got the tests running.

PTAL.

Thanks,

https://codereview.chromium.org/275773002/diff/20001/components/bookmarks/cor...
File
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc
(right):

https://codereview.chromium.org/275773002/diff/20001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:39:
scoped_ptr<base::MessageLoop> message_loop_;
On 2014/05/09 07:42:53, blundell wrote:
> This can just be a base::MessageLoop.

Done.

https://codereview.chromium.org/275773002/diff/20001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:63:
ui_task_runner_);
On 2014/05/09 07:42:53, blundell wrote:
> Don't create a separate ui_task_runner_; pass MessageLoopProxy::current().

Done.

[blundell, 2014-05-12 08:04:47]

LGTM - nice!

https://codereview.chromium.org/275773002/diff/60001/components/bookmarks/cor...
File
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc
(right):

https://codereview.chromium.org/275773002/diff/60001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:55:
prefs_ = PrefServiceForTesting().Pass();
nit: The .Pass() shouldn't be necessary here.

https://codereview.chromium.org/275773002/diff/60001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:67:
base::RunLoop().RunUntilIdle();
out of curiosity, what is this needed for? there was some DCHECK/crash otherwise
I assume?

[tfarina, 2014-05-12 18:18:27]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-12 18:18:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tfarina@chromium.org/275773002/80001

[tfarina, 2014-05-12 18:20:20]

https://codereview.chromium.org/275773002/diff/60001/components/bookmarks/cor...
File
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc
(right):

https://codereview.chromium.org/275773002/diff/60001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:55:
prefs_ = PrefServiceForTesting().Pass();
On 2014/05/12 08:04:48, blundell wrote:
> nit: The .Pass() shouldn't be necessary here.

Done.

https://codereview.chromium.org/275773002/diff/60001/components/bookmarks/cor...
components/bookmarks/core/browser/bookmark_expanded_state_tracker_unittest.cc:67:
base::RunLoop().RunUntilIdle();
On 2014/05/12 08:04:48, blundell wrote:
> out of curiosity, what is this needed for? there was some DCHECK/crash
otherwise
> I assume?
Tested without it now, and it passed.

[tfarina, 2014-05-12 18:55:26]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-12 18:56:20]

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

[commit-bot: I haz the power, 2014-05-12 22:30:08]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-12 23:01:56]

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

[commit-bot: I haz the power, 2014-05-12 23:01:57]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[tfarina, 2014-05-12 23:54:04]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-05-12 23:56:42]

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

[commit-bot: I haz the power, 2014-05-12 23:59:05]

Change committed as 269923