Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(887)

Issue 2698783003: [PlzNavigate] Notify inspector about client navigations to retain initiators. (Closed)

Created:
3 years, 10 months ago by dgozman
Modified:
3 years, 10 months ago
Reviewers:
clamy, pfeldman
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, kozyatinskiy+blink_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[PlzNavigate] Notify inspector about client navigations to retain initiators. With browser side navigation, the moment navigation is requested (and when the initiator should be collected) is different from the moment the request is sent in the renderer. We should collect initiator and store it until the request is sent. This is very similar to how javascript navigations are deferred, so we use similar plumbing here. Unfortunately, we cannot reuse the exact same instrumentation since it's also exposed through protocol. BUG=551000 Review-Url: https://codereview.chromium.org/2698783003 Cr-Commit-Position: refs/heads/master@{#451431} Committed: https://chromium.googlesource.com/chromium/src/+/1db4ba10e0856b48462d15a83b2d8804bb473b49

Patch Set 1 #

Total comments: 4

Patch Set 2 : win x64 compile #

Messages

Total messages: 34 (15 generated)
dgozman
Could you please take a look?
3 years, 10 months ago (2017-02-16 00:19:29 UTC) #2
pfeldman
https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp (right): https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp#newcode1519 third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp:1519: String frameId = IdentifiersFactory::frameId(frame); Did you enumarate all the ...
3 years, 10 months ago (2017-02-16 01:45:26 UTC) #3
dgozman
https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp File third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp (right): https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp#newcode1519 third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp:1519: String frameId = IdentifiersFactory::frameId(frame); On 2017/02/16 01:45:26, pfeldman wrote: ...
3 years, 10 months ago (2017-02-16 05:29:36 UTC) #4
clamy
Thanks! One question below. https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h File third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h (right): https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h#newcode169 third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h:169: void frameScheduledClientNavigation(LocalFrame*); Sanity check: this ...
3 years, 10 months ago (2017-02-16 13:50:03 UTC) #5
dgozman
https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h File third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h (right): https://codereview.chromium.org/2698783003/diff/1/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h#newcode169 third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h:169: void frameScheduledClientNavigation(LocalFrame*); On 2017/02/16 13:50:02, clamy wrote: > Sanity ...
3 years, 10 months ago (2017-02-16 15:22:33 UTC) #6
clamy
Thanks! Lgtm.
3 years, 10 months ago (2017-02-16 15:43:45 UTC) #7
pfeldman
lgtm
3 years, 10 months ago (2017-02-16 21:27:41 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698783003/1
3 years, 10 months ago (2017-02-16 21:29:06 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/368302)
3 years, 10 months ago (2017-02-16 23:53:53 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698783003/1
3 years, 10 months ago (2017-02-17 00:58:15 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/368456)
3 years, 10 months ago (2017-02-17 02:12:08 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698783003/1
3 years, 10 months ago (2017-02-17 02:40:49 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/368511)
3 years, 10 months ago (2017-02-17 03:49:32 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698783003/20001
3 years, 10 months ago (2017-02-17 21:22:23 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on ...
3 years, 10 months ago (2017-02-17 23:26:37 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698783003/20001
3 years, 10 months ago (2017-02-18 01:03:35 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
3 years, 10 months ago (2017-02-18 03:05:46 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2698783003/20001
3 years, 10 months ago (2017-02-18 04:54:54 UTC) #31
commit-bot: I haz the power
3 years, 10 months ago (2017-02-18 06:42:01 UTC) #34
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/1db4ba10e0856b48462d15a83b2d...

Powered by Google App Engine
This is Rietveld 408576698