Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

TestSimpleTaskRunner::GetPendingTasks copies whole set of the pending
task for each invocation. That prevents a migration to the non-copyable
Callback.
This CL replaces it with newly added TakePendingTasks and others.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/2d1f91c1ed29c73b81cf6a037435489242bb7aa3
Cr-Commit-Position: refs/heads/master@{#422699}

[tzik, 2016-09-13 08:42:30]

Description was changed from

==========
Replace TestSimpleTaskRunner::GetPendingTasks with ReplacePendingTasks

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

to

==========
Replace TestSimpleTaskRunner::GetPendingTasks with ReplacePendingTasks

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[tzik, 2016-09-13 08:42:57]

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

[commit-bot: I haz the power, 2016-09-13 08:43:17]

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-09-13 08:54:56]

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

[commit-bot: I haz the power, 2016-09-13 08:54:57]

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

[tzik, 2016-09-13 09:14:16]

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

[commit-bot: I haz the power, 2016-09-13 09:14:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-13 09:27:53]

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

[commit-bot: I haz the power, 2016-09-13 09:28: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, 2016-09-13 09:49:34]

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

[commit-bot: I haz the power, 2016-09-13 09:49:35]

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

[tzik, 2016-09-13 10:35:25]

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

[commit-bot: I haz the power, 2016-09-13 10:35:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-13 11:14:16]

Description was changed from

==========
Replace TestSimpleTaskRunner::GetPendingTasks with ReplacePendingTasks

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-09-13 11:16:51]

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

[commit-bot: I haz the power, 2016-09-13 11:16:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tzik, 2016-09-13 11:42:33]

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

[commit-bot: I haz the power, 2016-09-13 11:42:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-13 11:43:20]

Patchset #1 (id:1) has been deleted

[tzik, 2016-09-13 11:43:33]

Patchset #1 (id:20001) has been deleted

[tzik, 2016-09-13 11:43:46]

Patchset #1 (id:40001) has been deleted

[commit-bot: I haz the power, 2016-09-13 12:26:46]

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

[commit-bot: I haz the power, 2016-09-13 12:26:47]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[tzik, 2016-09-13 12:29:41]

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

[commit-bot: I haz the power, 2016-09-13 12:29:49]

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-09-13 13:22:06]

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

[commit-bot: I haz the power, 2016-09-13 13:22:07]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tzik, 2016-09-21 15:13:22]

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

[commit-bot: I haz the power, 2016-09-21 15:13:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-21 15:18:11]

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

[commit-bot: I haz the power, 2016-09-21 15:18:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-21 17:07:49]

Description was changed from

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

TestSimpleTaskRunner::GetPendingTasks copies whole set of the pending
task for each invocation. That prevents a migration to the non-copyable
Callback.
This CL replaces it with newly added TakePendingTasks and others.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[tzik, 2016-09-21 17:10:39]

tzik@chromium.org changed reviewers:
+ rockot@chromium.org

[tzik, 2016-09-21 17:11:31]

tzik@chromium.org changed reviewers:
+ pastarmovj@chromium.org

[tzik, 2016-09-21 17:13:02]

tzik@chromium.org changed reviewers:
+ danakj@chromium.org

[tzik, 2016-09-21 17:16:04]

PTAL to:
danakj: //base, //cc
pastarmovj: policy/
rockot: //extensions, //device

[commit-bot: I haz the power, 2016-09-21 19:24:10]

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

[commit-bot: I haz the power, 2016-09-21 19:24:11]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-09-21 22:03:55]

lgtm

[danakj, 2016-09-21 22:16:59]

https://codereview.chromium.org/2331423002/diff/140001/base/test/test_simple_...
File base/test/test_simple_task_runner.h (right):

https://codereview.chromium.org/2331423002/diff/140001/base/test/test_simple_...
base/test/test_simple_task_runner.h:59: tracked_objects::Location
GetPendingTaskLocationAt(size_t index) const;
I don't think this is safe, in the time between NumPendingTasks and the call to
this, the index may not be valid anymore since you're not holding the |lock_|?

[pastarmovj, 2016-09-22 08:12:51]

lgtm for the policy changes.

https://codereview.chromium.org/2331423002/diff/140001/base/test/test_simple_...
File base/test/test_simple_task_runner.h (right):

https://codereview.chromium.org/2331423002/diff/140001/base/test/test_simple_...
base/test/test_simple_task_runner.h:59: tracked_objects::Location
GetPendingTaskLocationAt(size_t index) const;
On 2016/09/21 22:16:59, danakj wrote:
> I don't think this is safe, in the time between NumPendingTasks and the call
to
> this, the index may not be valid anymore since you're not holding the |lock_|?

+1 

Maybe take a snapshot of the deque with TakePendingTasks and assert its size and
contents instead. Then you don't need this function at all.

[tzik, 2016-09-29 04:39:49]

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

[commit-bot: I haz the power, 2016-09-29 04:40:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-29 05:58:04]

https://codereview.chromium.org/2331423002/diff/140001/base/test/test_simple_...
File base/test/test_simple_task_runner.h (right):

https://codereview.chromium.org/2331423002/diff/140001/base/test/test_simple_...
base/test/test_simple_task_runner.h:59: tracked_objects::Location
GetPendingTaskLocationAt(size_t index) const;
On 2016/09/21 22:16:59, danakj wrote:
> I don't think this is safe, in the time between NumPendingTasks and the call
to
> this, the index may not be valid anymore since you're not holding the |lock_|?

Ah, right. I updated it to return a base::Optional. Other data races should be
handled by the caller.

[commit-bot: I haz the power, 2016-09-29 05:59:36]

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

[commit-bot: I haz the power, 2016-09-29 05:59:37]

Dry run: This issue passed the CQ dry run.

[danakj, 2016-09-29 19:42:14]

https://codereview.chromium.org/2331423002/diff/160001/extensions/common/one_...
File extensions/common/one_shot_event_unittest.cc (right):

https://codereview.chromium.org/2331423002/diff/160001/extensions/common/one_...
extensions/common/one_shot_event_unittest.cc:38:
runner->GetPendingTaskLocationAt(0);
Proposal: Rewrite this test to TakePendingTasks instead. Add second test that
runs the tasks and verifies i == 2. Remove this function from the API.

[tzik, 2016-09-30 03:59:58]

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

[commit-bot: I haz the power, 2016-09-30 04:00:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tzik, 2016-09-30 04:00:27]

https://codereview.chromium.org/2331423002/diff/160001/extensions/common/one_...
File extensions/common/one_shot_event_unittest.cc (right):

https://codereview.chromium.org/2331423002/diff/160001/extensions/common/one_...
extensions/common/one_shot_event_unittest.cc:38:
runner->GetPendingTaskLocationAt(0);
On 2016/09/29 19:42:13, danakj wrote:
> Proposal: Rewrite this test to TakePendingTasks instead. Add second test that
> runs the tasks and verifies i == 2. Remove this function from the API.

SG. Updated.

[commit-bot: I haz the power, 2016-09-30 06:07:39]

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

[commit-bot: I haz the power, 2016-09-30 06:07:40]

Dry run: This issue passed the CQ dry run.

[danakj, 2016-09-30 19:31:40]

LGTM

[tzik, 2016-10-04 01:59:00]

The CQ bit was checked by tzik@chromium.org

[tzik, 2016-10-04 01:59:01]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org,
pastarmovj@chromium.org
Link to the patchset: https://codereview.chromium.org/2331423002/#ps180001
(title: "split test")

[commit-bot: I haz the power, 2016-10-04 01:59:23]

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-10-04 03:58:24]

Description was changed from

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

TestSimpleTaskRunner::GetPendingTasks copies whole set of the pending
task for each invocation. That prevents a migration to the non-copyable
Callback.
This CL replaces it with newly added TakePendingTasks and others.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

TestSimpleTaskRunner::GetPendingTasks copies whole set of the pending
task for each invocation. That prevents a migration to the non-copyable
Callback.
This CL replaces it with newly added TakePendingTasks and others.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-10-04 03:58:25]

Committed patchset #7 (id:180001)

[commit-bot: I haz the power, 2016-10-04 04:02:03]

Description was changed from

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

TestSimpleTaskRunner::GetPendingTasks copies whole set of the pending
task for each invocation. That prevents a migration to the non-copyable
Callback.
This CL replaces it with newly added TakePendingTasks and others.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Replace TestSimpleTaskRunner::GetPendingTasks with TakePendingTasks

TestSimpleTaskRunner::GetPendingTasks copies whole set of the pending
task for each invocation. That prevents a migration to the non-copyable
Callback.
This CL replaces it with newly added TakePendingTasks and others.

BUG=554299
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/2d1f91c1ed29c73b81cf6a037435489242bb7aa3
Cr-Commit-Position: refs/heads/master@{#422699}
==========

[commit-bot: I haz the power, 2016-10-04 04:02:04]

Patchset 7 (id:??) landed as
https://crrev.com/2d1f91c1ed29c73b81cf6a037435489242bb7aa3
Cr-Commit-Position: refs/heads/master@{#422699}