Clear PendingScript::m_streamer when it is cancelled

Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() (introduced by
https://codereview.chromium.org/2268153002) cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
Committed: https://crrev.com/36a975bd72d1baaad0fe6dafc63496d7fdba0ba8
Cr-Commit-Position: refs/heads/master@{#435822}

[hiroshige, 2016-12-01 08:08:59]

Description was changed from

==========
Clear PendingScript::m_streamer when it is cancelled

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

to

==========
Clear PendingScript::m_streamer when it is cancelled



BUG=668611
==========

[hiroshige, 2016-12-01 08:09:15]

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

[commit-bot: I haz the power, 2016-12-01 08:09:44]

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

[hiroshige, 2016-12-01 08:14:48]

Description was changed from

==========
Clear PendingScript::m_streamer when it is cancelled



BUG=668611
==========

to

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
==========

[hiroshige, 2016-12-01 08:14:48]

hiroshige@chromium.org changed reviewers:
+ kouhei@chromium.org, marja@chromium.org, tasak@google.com

[hiroshige, 2016-12-01 08:16:00]

Description was changed from

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
==========

to

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() (introduced by
https://codereview.chromium.org/2268153002) cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
==========

[hiroshige, 2016-12-01 09:01:16]

PTAL.

[marja, 2016-12-01 09:03:19]

lgtm

Is it possible to add a test (as a follow up CL) demonstrating what the problem
was and that it's now fixed?

[commit-bot: I haz the power, 2016-12-01 09:26:08]

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

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

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-12-01 09:41:28]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2016-12-01 09:41:39]

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

[hiroshige, 2016-12-01 09:45:02]

On 2016/12/01 09:03:19, marja wrote:
> lgtm
> 
> Is it possible to add a test (as a follow up CL) demonstrating what the
problem
> was and that it's now fixed?

I think tests are needed to cover the MemoryCoordinator/onMemoryStateChange()
paths.
tasak@ left a comment about testing in the CL introduced the function
https://codereview.chromium.org/2268153002 (Comment #38).
tasak@, do you have any update after that?

[commit-bot: I haz the power, 2016-12-01 09:47:57]

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

[commit-bot: I haz the power, 2016-12-01 09:47:58]

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...)

[dominicc (has gone to gerrit), 2016-12-02 02:00:58]

dominicc@chromium.org changed reviewers:
+ dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-12-02 02:00:58]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-12-02 02:00:59]

lgtm

[commit-bot: I haz the power, 2016-12-02 02:01:33]

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-12-02 02:06:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480644058696440,
"parent_rev": "bb642a631687a42b7cc3368bc2e8e4a597fdd120", "commit_rev":
"9780f01bcb773d6082d8424576b59745b7c4abf9"}

[commit-bot: I haz the power, 2016-12-02 02:06:57]

Description was changed from

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() (introduced by
https://codereview.chromium.org/2268153002) cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
==========

to

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() (introduced by
https://codereview.chromium.org/2268153002) cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
==========

[commit-bot: I haz the power, 2016-12-02 02:06:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-02 02:12:49]

Description was changed from

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() (introduced by
https://codereview.chromium.org/2268153002) cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611
==========

to

==========
Clear PendingScript::m_streamer when it is cancelled

ScriptStreamer::notifyAppendData() should not called when ScriptStreamer is
cancelled.
However, PendingScript::onMemoryStateChange() (introduced by
https://codereview.chromium.org/2268153002) cancels ScriptStreamer but
leaves |m_streamer| non-null, potentially causing following calls to
notifyAppendData().

This CL clears |m_streamer| there, and also replaces DCHECKs with CHECKs
about resource pointers in ScriptStreamer methods with CHECKs to confirm
in the wild ScriptStreamer is not cancelled and the correct |m_resource|
value is held when the methods are called.

BUG=668611

Committed: https://crrev.com/36a975bd72d1baaad0fe6dafc63496d7fdba0ba8
Cr-Commit-Position: refs/heads/master@{#435822}
==========

[commit-bot: I haz the power, 2016-12-02 02:12:49]

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

[hiroshige, 2016-12-06 05:40:35]

On 2016/12/01 09:03:19, marja wrote:
> lgtm
> 
> Is it possible to add a test (as a follow up CL) demonstrating what the
problem
> was and that it's now fixed?

Filed crbug.com/671502 for the testing issue.