|
|
DescriptionTaskScheduler: Adjust shutdown behavior of delayed tasks.
With this CL, the shutdown behavior of a task posted with a delay to a
BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN.
This ensures that a delayed task never blocks shutdown before being
scheduled. This matches the behavior of SequencedWorkerPool.
TBR=danakj@chromium.org
BUG=553459
Committed: https://crrev.com/ad44b092b755176fc0f57ad0330efa2944d22a94
Cr-Commit-Position: refs/heads/master@{#422541}
Patch Set 1 #
Total comments: 11
Patch Set 2 : no copy #
Total comments: 6
Patch Set 3 : CR gab #10 #
Total comments: 5
Patch Set 4 : CR robliao #14 #
Total comments: 1
Dependent Patchsets: Messages
Total messages: 45 (19 generated)
fdoray@chromium.org changed reviewers: + gab@chromium.org, robliao@chromium.org
PTAL
https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc... base/task_scheduler/task.cc:28: TimeDelta delay) This should remain a const ref. AdjustShutdownBehavior can also adjust to be a const ref as well it appears. https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task_un... File base/task_scheduler/task_unittest.cc (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task_un... base/task_scheduler/task_unittest.cc:16: // Verify that iff a task is created with a delay and a BLOCK_SHUTDOWN shutdown s/iff/if/ https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task_un... base/task_scheduler/task_unittest.cc:39: Task continue_on_shutdown_delayed( This can be separated into two tests NoBehaviorChangeNoDelay ShutdownBehaviorChangeWithDelay If you've got better names, feel free to use them :-)
https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc... base/task_scheduler/task.cc:33: traits(AdjustShutdownBehavior(traits, delay)) {} So this potentially results in 3 copies (one to pass argument, one for return value, on into |traits|), instead of a single one into |traits|. Copy-eliding (since the return value is an rvalue) probably makes this only 2 copies and inlining could potentially bring it back to one but neither is guaranteed. Is it necessary to adjust the |traits| or should TaskTracker handle this on its own (it knows both delay and shutdown behavior)? https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h File base/task_scheduler/task.h (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h#... base/task_scheduler/task.h:33: TimeDelta delay); Is passing TimeDelta by value common practice? I know it's fine in practice because it's merely an int64_t but style is usually still to pass classes by const&.
https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc... base/task_scheduler/task.cc:28: TimeDelta delay) On 2016/09/30 18:57:20, robliao wrote: > This should remain a const ref. > > AdjustShutdownBehavior can also adjust to be a const ref as well it appears. No. https://cs.chromium.org/chromium/src/base/time/time.h?l=24 https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.cc... base/task_scheduler/task.cc:33: traits(AdjustShutdownBehavior(traits, delay)) {} On 2016/09/30 19:01:26, gab wrote: > So this potentially results in 3 copies (one to pass argument, one for return > value, on into |traits|), instead of a single one into |traits|. > > Copy-eliding (since the return value is an rvalue) probably makes this only 2 > copies and inlining could potentially bring it back to one but neither is > guaranteed. > > Is it necessary to adjust the |traits| or should TaskTracker handle this on its > own (it knows both delay and shutdown behavior)? I'll make a new CL that changes TaskTracker instead of Task. https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h File base/task_scheduler/task.h (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h#... base/task_scheduler/task.h:33: TimeDelta delay); On 2016/09/30 19:01:26, gab wrote: > Is passing TimeDelta by value common practice? I know it's fine in practice > because it's merely an int64_t but style is usually still to pass classes by > const&. See https://cs.chromium.org/chromium/src/base/time/time.h?l=24 TaskRunner takes a TimeDelta by value.
https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h File base/task_scheduler/task.h (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h#... base/task_scheduler/task.h:33: TimeDelta delay); On 2016/09/30 19:10:11, fdoray wrote: > On 2016/09/30 19:01:26, gab wrote: > > Is passing TimeDelta by value common practice? I know it's fine in practice > > because it's merely an int64_t but style is usually still to pass classes by > > const&. > > See https://cs.chromium.org/chromium/src/base/time/time.h?l=24 > TaskRunner takes a TimeDelta by value. I don't think there is a style ruling on passing by reference vs by value. The only requirement is if you pass by reference, you make it a const reference. There is a strong convention that generally input args are const reference, but not absolutely required. In these cases, I sometimes let the code vote, and it looks like TimeDelta (by value) wins.
Description was changed from ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. BUG=553459 ========== to ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. BUG=553459 ==========
FTR https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h File base/task_scheduler/task.h (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task.h#... base/task_scheduler/task.h:33: TimeDelta delay); On 2016/09/30 19:16:56, robliao wrote: > On 2016/09/30 19:10:11, fdoray wrote: > > On 2016/09/30 19:01:26, gab wrote: > > > Is passing TimeDelta by value common practice? I know it's fine in practice > > > because it's merely an int64_t but style is usually still to pass classes by > > > const&. > > > > See https://cs.chromium.org/chromium/src/base/time/time.h?l=24 > > TaskRunner takes a TimeDelta by value. > > I don't think there is a style ruling on passing by reference vs by value. The > only requirement is if you pass by reference, you make it a const reference. > > There is a strong convention that generally input args are const reference, but > not absolutely required. > > In these cases, I sometimes let the code vote, and it looks like TimeDelta (by > value) wins. IMO by value only wins because of https://cs.chromium.org/chromium/src/base/time/time.h?l=24. i.e. only if the class provides documented guarantees, otherwise you can't assume it won't grow beyond POD types only and become inefficient to pass by value.
PTAnL https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task_un... File base/task_scheduler/task_unittest.cc (right): https://codereview.chromium.org/2383193002/diff/1/base/task_scheduler/task_un... base/task_scheduler/task_unittest.cc:16: // Verify that iff a task is created with a delay and a BLOCK_SHUTDOWN shutdown On 2016/09/30 18:57:20, robliao wrote: > s/iff/if/ Done.
lgtm % comments https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: this->traits.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); rm "this->" ? To avoid naming conflict and make sure you're referring to the right one, I've previously preferred suffixing the params with "_in" for structs, e.g. : https://cs.chromium.org/chromium/src/chrome/installer/util/shell_util.h?rcl=0... https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/task.h File base/task_scheduler/task.h (right): https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task.h:29: // automatically changed to SKIP_ON_SHUTDOWN. s/changed/adjusted/ https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... File base/task_scheduler/task_unittest.cc (right): https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task_unittest.cc:43: TEST(TaskSchedulerTaskTest, ShutdownBehaviorChangeWithDelay) { The test names are backwards (i.e. this one should be "NoDelay")
robliao@: PTAL https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: this->traits.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); On 2016/10/03 16:59:53, gab wrote: > rm "this->" ? > > To avoid naming conflict and make sure you're referring to the right one, I've > previously preferred suffixing the params with "_in" for structs, e.g. : > https://cs.chromium.org/chromium/src/chrome/installer/util/shell_util.h?rcl=0... Done. https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/task.h File base/task_scheduler/task.h (right): https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task.h:29: // automatically changed to SKIP_ON_SHUTDOWN. On 2016/10/03 16:59:53, gab wrote: > s/changed/adjusted/ Done. https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... File base/task_scheduler/task_unittest.cc (right): https://codereview.chromium.org/2383193002/diff/20001/base/task_scheduler/tas... base/task_scheduler/task_unittest.cc:43: TEST(TaskSchedulerTaskTest, ShutdownBehaviorChangeWithDelay) { On 2016/10/03 16:59:53, gab wrote: > The test names are backwards (i.e. this one should be "NoDelay") Done.
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm + comments https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); Should this be traits instead of traits_in? Actually, I'm surprised this works given the const TaskTraits& traits_in. Also add a TODO to DCHECK this once all the callers are cleaned up.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); On 2016/10/03 17:16:35, robliao wrote: > Should this be traits instead of traits_in? > > Actually, I'm surprised this works given the const TaskTraits& traits_in. > > Also add a TODO to DCHECK this once all the callers are cleaned up. I uploaded the wrong patch. This should be |traits| instead of |traits_in|. Is the TODO still required since this code doesn't compile? It would also not have passed unit tests if the const for |traits_in| had been removed to make the code compile.
The CQ bit was checked by fdoray@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); On 2016/10/03 17:27:48, fdoray wrote: > On 2016/10/03 17:16:35, robliao wrote: > > Should this be traits instead of traits_in? > > > > Actually, I'm surprised this works given the const TaskTraits& traits_in. > > > > Also add a TODO to DCHECK this once all the callers are cleaned up. > > I uploaded the wrong patch. This should be |traits| instead of |traits_in|. > > Is the TODO still required since this code doesn't compile? It would also not > have passed unit tests if the const for |traits_in| had been removed to make the > code compile. I'm meant a TODO that we're rewriting the shutdown behavior as skip on shutdown. Long term, we don't want callers to fall into this category, right?
https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); On 2016/10/03 17:30:12, robliao wrote: > On 2016/10/03 17:27:48, fdoray wrote: > > On 2016/10/03 17:16:35, robliao wrote: > > > Should this be traits instead of traits_in? > > > > > > Actually, I'm surprised this works given the const TaskTraits& traits_in. > > > > > > Also add a TODO to DCHECK this once all the callers are cleaned up. > > > > I uploaded the wrong patch. This should be |traits| instead of |traits_in|. > > > > Is the TODO still required since this code doesn't compile? It would also not > > have passed unit tests if the const for |traits_in| had been removed to make > the > > code compile. > > I'm meant a TODO that we're rewriting the shutdown behavior as skip on shutdown. > Long term, we don't want callers to fall into this category, right? From our chat discussion, I thought we wanted to allow callers to post delayed tasks to a BLOCK_SHUTDOWN TaskRunner and automatically adjust shutdown behavior to SKIP_ON_SHTUDOWN (i.e. we don't want to change call sites). That's not the case?
On 2016/10/03 17:36:33, fdoray wrote: > https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... > File base/task_scheduler/task.cc (right): > > https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... > base/task_scheduler/task.cc:23: > traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); > On 2016/10/03 17:30:12, robliao wrote: > > On 2016/10/03 17:27:48, fdoray wrote: > > > On 2016/10/03 17:16:35, robliao wrote: > > > > Should this be traits instead of traits_in? > > > > > > > > Actually, I'm surprised this works given the const TaskTraits& traits_in. > > > > > > > > Also add a TODO to DCHECK this once all the callers are cleaned up. > > > > > > I uploaded the wrong patch. This should be |traits| instead of |traits_in|. > > > > > > Is the TODO still required since this code doesn't compile? It would also > not > > > have passed unit tests if the const for |traits_in| had been removed to make > > the > > > code compile. > > > > I'm meant a TODO that we're rewriting the shutdown behavior as skip on > shutdown. > > Long term, we don't want callers to fall into this category, right? > > From our chat discussion, I thought we wanted to allow callers to post delayed > tasks to a BLOCK_SHUTDOWN TaskRunner and automatically adjust shutdown behavior > to SKIP_ON_SHTUDOWN (i.e. we don't want to change call sites). > > That's not the case? Indeed, but once we have child task runners, folks should be able to do the right thing, right? Or do we want to keep the convenience after the conversion? I can see this go either way.
The CQ bit was unchecked by fdoray@chromium.org
The CQ bit was checked by fdoray@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org, robliao@chromium.org Link to the patchset: https://codereview.chromium.org/2383193002/#ps60001 (title: "CR robliao #14")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... base/task_scheduler/task.cc:23: traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); On 2016/10/03 17:36:33, fdoray wrote: > On 2016/10/03 17:30:12, robliao wrote: > > On 2016/10/03 17:27:48, fdoray wrote: > > > On 2016/10/03 17:16:35, robliao wrote: > > > > Should this be traits instead of traits_in? > > > > > > > > Actually, I'm surprised this works given the const TaskTraits& traits_in. > > > > > > > > Also add a TODO to DCHECK this once all the callers are cleaned up. > > > > > > I uploaded the wrong patch. This should be |traits| instead of |traits_in|. > > > > > > Is the TODO still required since this code doesn't compile? It would also > not > > > have passed unit tests if the const for |traits_in| had been removed to make > > the > > > code compile. > > > > I'm meant a TODO that we're rewriting the shutdown behavior as skip on > shutdown. > > Long term, we don't want callers to fall into this category, right? > > From our chat discussion, I thought we wanted to allow callers to post delayed > tasks to a BLOCK_SHUTDOWN TaskRunner and automatically adjust shutdown behavior > to SKIP_ON_SHTUDOWN (i.e. we don't want to change call sites). > > That's not the case? Discussed offline: We'll go ahead and keep this long term and reassess later.
On 2016/10/03 17:39:20, robliao wrote: > On 2016/10/03 17:36:33, fdoray wrote: > > > https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... > > File base/task_scheduler/task.cc (right): > > > > > https://codereview.chromium.org/2383193002/diff/40001/base/task_scheduler/tas... > > base/task_scheduler/task.cc:23: > > traits_in.WithShutdownBehavior(TaskShutdownBehavior::SKIP_ON_SHUTDOWN); > > On 2016/10/03 17:30:12, robliao wrote: > > > On 2016/10/03 17:27:48, fdoray wrote: > > > > On 2016/10/03 17:16:35, robliao wrote: > > > > > Should this be traits instead of traits_in? > > > > > > > > > > Actually, I'm surprised this works given the const TaskTraits& > traits_in. > > > > > > > > > > Also add a TODO to DCHECK this once all the callers are cleaned up. > > > > > > > > I uploaded the wrong patch. This should be |traits| instead of > |traits_in|. > > > > > > > > Is the TODO still required since this code doesn't compile? It would also > > not > > > > have passed unit tests if the const for |traits_in| had been removed to > make > > > the > > > > code compile. > > > > > > I'm meant a TODO that we're rewriting the shutdown behavior as skip on > > shutdown. > > > Long term, we don't want callers to fall into this category, right? > > > > From our chat discussion, I thought we wanted to allow callers to post delayed > > tasks to a BLOCK_SHUTDOWN TaskRunner and automatically adjust shutdown > behavior > > to SKIP_ON_SHTUDOWN (i.e. we don't want to change call sites). > > > > That's not the case? > > Indeed, but once we have child task runners, folks should be able to do the > right thing, right? > Or do we want to keep the convenience after the conversion? I can see this go > either way. Conclusion of chat discussion: - No TODO - Posting a delayed task to a BLOCK_SHUTDOWN TaskRunner is allowed. - We may reassess later.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. BUG=553459 ========== to ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. TBR=danakj@chromium.org BUG=553459 ==========
TBR danakj for changes in base/BUILD.gn
fdoray@chromium.org changed reviewers: + danakj@chromium.org
TBR danakj for changes in base/BUILD.gn
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by fdoray@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM https://codereview.chromium.org/2383193002/diff/60001/base/task_scheduler/tas... File base/task_scheduler/task.cc (right): https://codereview.chromium.org/2383193002/diff/60001/base/task_scheduler/tas... base/task_scheduler/task.cc:21: if (!delay.is_zero() && Maybe you could put this into a constexpr helper and keep the member as const.
Message was sent while issue was closed.
Description was changed from ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. TBR=danakj@chromium.org BUG=553459 ========== to ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. TBR=danakj@chromium.org BUG=553459 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. TBR=danakj@chromium.org BUG=553459 ========== to ========== TaskScheduler: Adjust shutdown behavior of delayed tasks. With this CL, the shutdown behavior of a task posted with a delay to a BLOCK_SHUTDOWN TaskRunner is automatically adjusted to SKIP_ON_SHUTDOWN. This ensures that a delayed task never blocks shutdown before being scheduled. This matches the behavior of SequencedWorkerPool. TBR=danakj@chromium.org BUG=553459 Committed: https://crrev.com/ad44b092b755176fc0f57ad0330efa2944d22a94 Cr-Commit-Position: refs/heads/master@{#422541} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/ad44b092b755176fc0f57ad0330efa2944d22a94 Cr-Commit-Position: refs/heads/master@{#422541} |