webRequest: Set frameId on beacon/ping requests.

webRequest: Set frameId on beacon/ping requests.

BUG=522124, 522129
Committed: https://crrev.com/ac7ee4ce7b8d39b22a710c58d110e0039c11cf9a
Cr-Commit-Position: refs/heads/master@{#397357}

[robwu, 2016-05-22 21:03:57]

rob@robwu.nl changed reviewers:
+ japhet@chromium.org

[Nate Chapin, 2016-05-23 21:28:04]

Seems reasonable, just a cleanup suggestion.

https://codereview.chromium.org/1997253002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/PingLoader.cpp (right):

https://codereview.chromium.org/1997253002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/PingLoader.cpp:142:
frame->loader().client()->dispatchWillSendRequest(frame->loader().documentLoader(),
m_identifier, request, redirectResponse);
Could we call frame->document()->fetcher()->context().dispatchWillSendRequest()
instead of doing this and the 2 lines below? This code appears to be
unnecessarily (and until your change, incorrectly!) duplicated.

[dcheng, 2016-05-23 21:30:07]

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

[dcheng, 2016-05-23 21:30:07]

Hmm. How important is it that the unique identifier is actually unique?

[robwu, 2016-05-23 22:28:31]

On 2016/05/23 21:30:07, dcheng wrote:
> Hmm. How important is it that the unique identifier is actually unique?

I don't know - I took a quick look and found that |identifier| is used by test
code, but not by RenderFrameImpl::willSendRequest.

https://codereview.chromium.org/1997253002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/loader/PingLoader.cpp (right):

https://codereview.chromium.org/1997253002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/loader/PingLoader.cpp:142:
frame->loader().client()->dispatchWillSendRequest(frame->loader().documentLoader(),
m_identifier, request, redirectResponse);
On 2016/05/23 21:28:04, Nate Chapin wrote:
> Could we call
frame->document()->fetcher()->context().dispatchWillSendRequest()
> instead of doing this and the 2 lines below? This code appears to be
> unnecessarily (and until your change, incorrectly!) duplicated.

Done.

[Nate Chapin, 2016-06-01 23:58:13]

Sorry for the delay, blink LGTM

[robwu, 2016-06-02 08:29:51]

The CQ bit was checked by rob@robwu.nl

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

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1997253002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1997253002/60001

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

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-02 10:49:08]

Description was changed from

==========
webRequest: Set frameId on beacon/ping requests.

BUG=522124,522129
==========

to

==========
webRequest: Set frameId on beacon/ping requests.

BUG=522124,522129

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

[commit-bot: I haz the power, 2016-06-02 10:49:09]

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