[heap] Reland "Cancel tasks before tearing down the heap."

[heap] Reland "Cancel tasks before tearing down the heap."

BUG=chromium:654343
Committed: https://crrev.com/ddf83fb84a74eb813396fa7b1fa83a3064a03878
Cr-Commit-Position: refs/heads/master@{#40305}

[ulan, 2016-10-14 08:23:33]

ulan@chromium.org changed reviewers:
+ mlippautz@chromium.org

[ulan, 2016-10-14 08:23:34]

ptal

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.cc
File src/cancelable-task.cc (right):

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.cc#newc...
src/cancelable-task.cc:37: CHECK(!canceled_);
This differs from the original CL. I think it is better to assert so that we can
get stack trace in case of Register being called after CancelAndWait.

[Michael Lippautz, 2016-10-14 09:29:20]

lgtm (only nits)

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.cc
File src/cancelable-task.cc (right):

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.cc#newc...
src/cancelable-task.cc:37: CHECK(!canceled_);
On 2016/10/14 08:23:34, ulan wrote:
> This differs from the original CL. I think it is better to assert so that we
can
> get stack trace in case of Register being called after CancelAndWait.

Acknowledged.

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.h
File src/cancelable-task.h (right):

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.h#newco...
src/cancelable-task.h:30: // Should not be called after CancelAndWait.
s/Should/Must/

(since it's a CHECK now)

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.h#newco...
src/cancelable-task.h:44: // already running.
Maybe add a comment here that this freezes/seals/<other fancy term> the manager
object so you cannot register anymore.

[ulan, 2016-10-14 09:34:35]

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.h
File src/cancelable-task.h (right):

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.h#newco...
src/cancelable-task.h:30: // Should not be called after CancelAndWait.
On 2016/10/14 09:29:20, Michael Lippautz wrote:
> s/Should/Must/
> 
> (since it's a CHECK now)

Done.

https://codereview.chromium.org/2419783004/diff/1/src/cancelable-task.h#newco...
src/cancelable-task.h:44: // already running.
On 2016/10/14 09:29:20, Michael Lippautz wrote:
> Maybe add a comment here that this freezes/seals/<other fancy term> the
manager
> object so you cannot register anymore.

Done.

[ulan, 2016-10-14 09:34:48]

The CQ bit was checked by ulan@chromium.org

[ulan, 2016-10-14 09:34:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org
Link to the patchset: https://codereview.chromium.org/2419783004/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2016-10-14 09:34:56]

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-14 10:07:25]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-14 10:07:41]

Description was changed from

==========
[heap] Reland "Cancel tasks before tearing down the heap."

BUG=chromium:654343
==========

to

==========
[heap] Reland "Cancel tasks before tearing down the heap."

BUG=chromium:654343

Committed: https://crrev.com/ddf83fb84a74eb813396fa7b1fa83a3064a03878
Cr-Commit-Position: refs/heads/master@{#40305}
==========

[commit-bot: I haz the power, 2016-10-14 10:07:42]

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