New issue
Advanced search Search tips

Issue 683900 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

PingLoader should not use the per-frame task queue

Project Member Reported by csharrison@chromium.org, Jan 23 2017

Issue description

This could cause hanging posts, where the objects timeout is never reached.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/49db2761173c14255f053871fb0b862dcf57fda3

commit 49db2761173c14255f053871fb0b862dcf57fda3
Author: csharrison <csharrison@chromium.org>
Date: Mon Jan 23 15:39:38 2017

Revert of Use TaskRunnerTimer in PingLoader (patchset #1 id:1 of https://codereview.chromium.org/2646523002/ )

Reason for revert:
Pingloader should have its timeout persist after frame destruction.

Original issue's description:
> Use TaskRunnerTimer in PingLoader
>
> Blocker for the per-frame scheduler.
>
> BUG= 624694 
>
> Review-Url: https://codereview.chromium.org/2646523002
> Cr-Commit-Position: refs/heads/master@{#444627}
> Committed: https://chromium.googlesource.com/chromium/src/+/81f01201993a78e1c89d70f11bc90b636a53cc99

TBR=yhirano@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 624694 , 683900 

Review-Url: https://codereview.chromium.org/2654463002
Cr-Commit-Position: refs/heads/master@{#445380}

[modify] https://crrev.com/49db2761173c14255f053871fb0b862dcf57fda3/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/49db2761173c14255f053871fb0b862dcf57fda3/third_party/WebKit/Source/core/loader/PingLoader.h

Cc: kinuko@chromium.org
Labels: Merge-Request-57
Requesting a merge to M57 because this change landed before branch point.

Comment 3 by gov...@chromium.org, Jan 23 2017

Is this change applicable to All OSs or any specific OS?
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
All OS except iOS.
Project Member

Comment 5 by sheriffbot@chromium.org, Jan 23 2017

Labels: -Merge-Request-57 Hotlist-Merge-Review Merge-Review-57
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by gov...@chromium.org, Jan 23 2017

Change listed at #1 is not yet baked in Canary. 
Please wait until change is well baked in Canary and update Canary result here so we can approve merge to M57. Thank you.


Comment 7 by gov...@chromium.org, Jan 25 2017

csharrison@, how is the revert looking in Canary?
It is looking fine.

Comment 9 by gov...@chromium.org, Jan 25 2017

Labels: -Merge-Review-57 Merge-Approved-57
Approving merge to M57 branch 2987 based on comment #8. Please merge ASAP. Thank you.
Status: Assigned (was: Untriaged)
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 25 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2d32dbc7514e959b3dd4c8b739a13d3f6d80a7b

commit d2d32dbc7514e959b3dd4c8b739a13d3f6d80a7b
Author: csharrison <csharrison@chromium.org>
Date: Wed Jan 25 18:28:18 2017

Revert of Use TaskRunnerTimer in PingLoader
(patchset #1 id:1 of https://codereview.chromium.org/2646523002/ )

Reason for revert:
Pingloader should have its timeout persist after frame destruction.

Original issue's description:
> Use TaskRunnerTimer in PingLoader
>
> Blocker for the per-frame scheduler.
>
> BUG= 624694 
>
> Review-Url: https://codereview.chromium.org/2646523002
> Cr-Commit-Position: refs/heads/master@{#444627}
> Committed: https://chromium.googlesource.com/chromium/src/+/81f01201993a78e1c89d70f11bc90b636a53cc99

TBR=yhirano@chromium.org, kinuko@chromium.org
BUG= 624694 , 683900 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2654463002
Cr-Commit-Position: refs/heads/master@{#445380}
(cherry picked from commit 49db2761173c14255f053871fb0b862dcf57fda3)

Review-Url: https://codereview.chromium.org/2652253003
Cr-Commit-Position: refs/branch-heads/2987@{#91}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/d2d32dbc7514e959b3dd4c8b739a13d3f6d80a7b/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/d2d32dbc7514e959b3dd4c8b739a13d3f6d80a7b/third_party/WebKit/Source/core/loader/PingLoader.h

Status: Fixed (was: Assigned)

Sign in to add a comment