Cancel RawResource asynchronously in allClientsAndObserversRemoved()

Cancel RawResource asynchronously in allClientsAndObserversRemoved()

allClientsAndObserversRemoved() can be called in subtle situations, e.g. in
prefinalizers, and thus we should avoid non-trivial things, such as Resource
cancelling.

This CL moves the last part of Resource cancelling (RawResource) out of
allClientsAndObserversRemoved().

BUG=606651, 644058
Committed: https://crrev.com/ab98db1717773d19bfae7ff23cac02c57b34ceae
Cr-Commit-Position: refs/heads/master@{#423472}

[hiroshige, 2016-09-15 05:22:03]

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

[commit-bot: I haz the power, 2016-09-15 05:22:14]

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-15 06:38:18]

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

[commit-bot: I haz the power, 2016-09-15 06:38:19]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-09-15 08:05:59]

Description was changed from

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

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

to

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

BUG=606651
==========

[hiroshige, 2016-09-15 08:05:59]

hiroshige@chromium.org changed reviewers:
+ japhet@chromium.org

[hiroshige, 2016-09-15 08:06:10]

PTAL.

[hiroshige, 2016-09-15 08:07:38]

Description was changed from

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

BUG=606651
==========

to

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

BUG=606651, 644058
==========

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

hiroshige@chromium.org changed reviewers:
+ yhirano@chromium.org

[yhirano, 2016-09-15 08:15:43]

On 2016/09/15 08:12:07, hiroshige wrote:

Can you add some more motivation / background to the description?
I cannot access to issue 606651.

[hiroshige, 2016-09-15 08:35:02]

Description was changed from

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

BUG=606651, 644058
==========

to

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

allClientsAndObserversRemoved() can be called in subtle situations, e.g. in
prefinalizers, and thus we should avoid non-trivial things, such as Resource
cancelling.

This CL moves the last part of Resource cancelling (RawResource) out of
allClientsAndObserversRemoved().

BUG=606651, 644058
==========

[hiroshige, 2016-09-15 08:35:48]

On 2016/09/15 08:15:43, yhirano (slow) wrote:
> On 2016/09/15 08:12:07, hiroshige wrote:
> 
> Can you add some more motivation / background to the description?
> I cannot access to issue 606651.

CC'ed you on issue 606651.
(RawResource cancelling is now not directly related to Issue 606651 though)

[Nate Chapin, 2016-09-16 21:27:03]

lgtm

[yhirano, 2016-09-20 07:20:39]

lgtm

[hiroshige, 2016-09-21 04:15:42]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2016-09-21 04:16:05]

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-21 04:24:42]

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

[commit-bot: I haz the power, 2016-09-21 04:24:43]

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

[hiroshige, 2016-09-28 04:32:46]

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

[commit-bot: I haz the power, 2016-09-28 04:32:52]

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-28 05:46:59]

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

[commit-bot: I haz the power, 2016-09-28 05:47:00]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-09-29 06:16:51]

hiroshige@chromium.org changed reviewers:
+ mkwst@chromium.org

[hiroshige, 2016-09-29 06:16:52]

+mkwst@, could you take a look?

[hiroshige, 2016-10-05 08:03:58]

friendly ping :)

[Mike West, 2016-10-05 12:35:33]

LGTM, sorry for the delay!

[hiroshige, 2016-10-06 03:10:29]

The CQ bit was checked by hiroshige@chromium.org

[commit-bot: I haz the power, 2016-10-06 03:10:41]

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-06 05:16:06]

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

[commit-bot: I haz the power, 2016-10-06 05:16:07]

Failed to apply patch for third_party/WebKit/Source/core/fetch/Resource.cpp:
While running git apply --index -3 -p1;
  error: patch failed: third_party/WebKit/Source/core/fetch/Resource.cpp:743
  Falling back to three-way merge...
  Applied patch to 'third_party/WebKit/Source/core/fetch/Resource.cpp' with
conflicts.
  U third_party/WebKit/Source/core/fetch/Resource.cpp

Patch:       third_party/WebKit/Source/core/fetch/Resource.cpp
Index: third_party/WebKit/Source/core/fetch/Resource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/Resource.cpp
b/third_party/WebKit/Source/core/fetch/Resource.cpp
index
9271ff3424ae4c4d85ef0c0b2bd035d2808f6228..15fdc012c5e4e278e6f5feeaf4fc12dc4f158151
100644
--- a/third_party/WebKit/Source/core/fetch/Resource.cpp
+++ b/third_party/WebKit/Source/core/fetch/Resource.cpp
@@ -743,9 +743,7 @@ void Resource::allClientsAndObserversRemoved()
 {
     if (!m_loader)
         return;
-    if (m_type == Raw)
-        cancelTimerFired(&m_cancelTimer);
-    else if (!m_cancelTimer.isActive())
+    if (!m_cancelTimer.isActive())
         m_cancelTimer.startOneShot(0, BLINK_FROM_HERE);
 }

[hiroshige, 2016-10-06 05:35:26]

The CQ bit was checked by hiroshige@chromium.org

[hiroshige, 2016-10-06 05:35:27]

The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org,
mkwst@chromium.org, yhirano@chromium.org
Link to the patchset: https://codereview.chromium.org/2306303003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-10-06 05:35:45]

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-06 07:55:48]

Description was changed from

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

allClientsAndObserversRemoved() can be called in subtle situations, e.g. in
prefinalizers, and thus we should avoid non-trivial things, such as Resource
cancelling.

This CL moves the last part of Resource cancelling (RawResource) out of
allClientsAndObserversRemoved().

BUG=606651, 644058
==========

to

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

allClientsAndObserversRemoved() can be called in subtle situations, e.g. in
prefinalizers, and thus we should avoid non-trivial things, such as Resource
cancelling.

This CL moves the last part of Resource cancelling (RawResource) out of
allClientsAndObserversRemoved().

BUG=606651, 644058
==========

[commit-bot: I haz the power, 2016-10-06 07:55:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-06 07:58:56]

Description was changed from

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

allClientsAndObserversRemoved() can be called in subtle situations, e.g. in
prefinalizers, and thus we should avoid non-trivial things, such as Resource
cancelling.

This CL moves the last part of Resource cancelling (RawResource) out of
allClientsAndObserversRemoved().

BUG=606651, 644058
==========

to

==========
Cancel RawResource asynchronously in allClientsAndObserversRemoved()

allClientsAndObserversRemoved() can be called in subtle situations, e.g. in
prefinalizers, and thus we should avoid non-trivial things, such as Resource
cancelling.

This CL moves the last part of Resource cancelling (RawResource) out of
allClientsAndObserversRemoved().

BUG=606651, 644058

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

[commit-bot: I haz the power, 2016-10-06 07:58:57]

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