Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad [chromium/src : master]
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 1:Commit-Queue +1
To view, visit change 542717. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki would like Yutaka Hirano to review this change.
Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad
Bug:
Change-Id: I3a9d14d8bb6ec35f9cb56b5dc567afa8f65f2e87
---
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/platform/WebFrameScheduler.h
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h
6 files changed, 18 insertions(+), 4 deletions(-)
Yutaka Hirano (Gerrit)
Yutaka Hirano posted comments on this change.
Patch set 1:Code-Review +1
Taiju Tsuiki (Gerrit)
Taiju Tsuiki uploaded patch set #2 to this change.
Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad
Blink Scheduler has been considering the renderer is the loading phase
after OnNavigate or DidCommitProvisionalLoad event. However on the
PlzNavigate case, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds DidStartProvisionalLoad as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: I3a9d14d8bb6ec35f9cb56b5dc567afa8f65f2e87
---
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/platform/WebFrameScheduler.h
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h
6 files changed, 18 insertions(+), 4 deletions(-)
To view, visit change 542717. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki uploaded patch set #3 to this change.
Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad
Blink Scheduler has been considering that the renderer is in the loading
phase after OnNavigate or DidCommitProvisionalLoad event. However on the
PlzNavigate case, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds DidStartProvisionalLoad as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: I3a9d14d8bb6ec35f9cb56b5dc567afa8f65f2e87
---
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/platform/WebFrameScheduler.h
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h
6 files changed, 18 insertions(+), 4 deletions(-)
To view, visit change 542717. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 3:
PTAL
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 3:
+japhet@ for reviewing (fake) provisional load timing in PlzNavigate case
(cc-ing Camille as well)
Camille Lamy (Gerrit)
Camille Lamy posted comments on this change.
Patch set 3:
Just a quick question based on the CL description: if the renderer considers that it is in a loading phase when we get OnNavigate, can we mirror that in PlzNavigate with the OnCommitNavigation IPC?
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 3:
Patch Set 3:
Just a quick question based on the CL description: if the renderer considers that it is in a loading phase when we get OnNavigate, can we mirror that in PlzNavigate with the OnCommitNavigation IPC?
Yeah that's what I initially suggested as well, might not have worked as expected? (Let's wait for Taiju's answer)
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 3:
Patch Set 3:
Patch Set 3:
Just a quick question based on the CL description: if the renderer considers that it is in a loading phase when we get OnNavigate, can we mirror that in PlzNavigate with the OnCommitNavigation IPC?
Yeah that's what I initially suggested as well, might not have worked as expected? (Let's wait for Taiju's answer)
That should work. I didn't do it just because I missed the name of the IPC and couldn't find it.
Let me update the CL tomorrow.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki uploaded patch set #5 to this change.
Notify RendererScheduler of starting LOADING phase on OnCommitNavigation
Blink Scheduler has been considering that the renderer is in the loading
phase after OnNavigate or DidCommitProvisionalLoad event. However on the
PlzNavigate case, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds OnCommitNavigation as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: I3a9d14d8bb6ec35f9cb56b5dc567afa8f65f2e87
---
M content/renderer/render_frame_impl.cc
1 file changed, 6 insertions(+), 0 deletions(-)
To view, visit change 542717. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 5:
Patch Set 3:
Patch Set 3:
Patch Set 3:
Just a quick question based on the CL description: if the renderer considers that it is in a loading phase when we get OnNavigate, can we mirror that in PlzNavigate with the OnCommitNavigation IPC?
Yeah that's what I initially suggested as well, might not have worked as expected? (Let's wait for Taiju's answer)
That should work. I didn't do it just because I missed the name of the IPC and couldn't find it.
Let me update the CL tomorrow.
Updated. It looks working on a local benchmark.
Yutaka Hirano (Gerrit)
Yutaka Hirano posted comments on this change.
Patch set 5:
How about the in-process navigation case with PlzNavigate disabled?
Camille Lamy (Gerrit)
Camille Lamy posted comments on this change.
Patch set 5:Code-Review +1
Thanks! Lgtm.
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 5:Code-Review +1
Cool, thanks! Just let me add one more stamp
Sami Kyöstilä (Gerrit)
Sami Kyöstilä posted comments on this change.
Patch set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 5:
Patch Set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Btw alternatively could we also look for better signals that just work?
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Btw alternatively could we also look for better signals that just work?
We can get navigation type in OnCommitNavigation from CommonNavigationParams, so I think we can only call OnNavigate for the types that are not for history navigation?
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I observed DidCommitProvisionalLoad call on a pushState/popState, but didn't see it on RenderFrameImpl::OnCommitNavigation nor FrameLoader::StartLoad. Maybe, the situation was changed over time?
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
To view, visit change 542717. To unsubscribe, visit settings.
Camille Lamy (Gerrit)
Camille Lamy posted comments on this change.
Patch set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Btw alternatively could we also look for better signals that just work?
We can get navigation type in OnCommitNavigation from CommonNavigationParams, so I think we can only call OnNavigate for the types that are not for history navigation?
We shouldn't exclude all history navigations, just same-document navigations in general. You can know that a navigation is intended as a same-document navigation by calling FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type).
Sami Kyöstilä (Gerrit)
Sami Kyöstilä posted comments on this change.
Patch set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Btw alternatively could we also look for better signals that just work?
We can get navigation type in OnCommitNavigation from CommonNavigationParams, so I think we can only call OnNavigate for the types that are not for history navigation?
We shouldn't exclude all history navigations, just same-document navigations in general. You can know that a navigation is intended as a same-document navigation by calling FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type).
If we can exclude same document navigations, we can probably just stick with the LOADING case. I was thinking we'd need to set different task and resource fetch priorities in NAVIGATING vs. LOADING but on further thought maybe that isn't necessary -- we'd just need to avoid throttling loading tasks in both cases, which means we don't need two distinct states.
Nate Chapin (Gerrit)
Nate Chapin posted comments on this change.
Patch set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Btw alternatively could we also look for better signals that just work?
We can get navigation type in OnCommitNavigation from CommonNavigationParams, so I think we can only call OnNavigate for the types that are not for history navigation?
We shouldn't exclude all history navigations, just same-document navigations in general. You can know that a navigation is intended as a same-document navigation by calling FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type).
If we can exclude same document navigations, we can probably just stick with the LOADING case. I was thinking we'd need to set different task and resource fetch priorities in NAVIGATING vs. LOADING but on further thought maybe that isn't necessary -- we'd just need to avoid throttling loading tasks in both cases, which means we don't need two distinct states.
Sorry to arrive late. I don't think OnCommitNavigation should ever be called for pushState/replaceState. That will definitely call DidCommitProvisionalLoad, but blink (and not PlzNavigate) should always be driving pushState/replaceState.
I *think* checking PlzNavigate's same document state should suffice to exclude all same-document navigations here.
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
Patch Set 5:
We used to have this hook in OnCommitNavigation but recently removed it because it was getting called for pushState()/popState(). The problem was that we ended up going into LOADING use case when those APIs were called but never came out of it because the page was already loaded from the first meaningful paint point of view (very apparent on youtube.com).
I understand knowing that we're about to generate fetches relating to a navigation is important for your work, so let's figure out a way to fit this in. Maybe we need a new use case for when we're navigating but haven't received the first byte of the main resource yet? We could call it NAVIGATING or something like that. WDYT?
Thanks for giving more context. Introducing NAVIGATING sounds good, but what'd be your proposal for the issues we're having? Would that be to add different signals to make use-case-migration happen from NAVIGATING that works for history navigations, and make the loading task runner not throttled during NAVIGATING state too? (If so it sgtm)
Btw alternatively could we also look for better signals that just work?
We can get navigation type in OnCommitNavigation from CommonNavigationParams, so I think we can only call OnNavigate for the types that are not for history navigation?
We shouldn't exclude all history navigations, just same-document navigations in general. You can know that a navigation is intended as a same-document navigation by calling FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type).
If we can exclude same document navigations, we can probably just stick with the LOADING case. I was thinking we'd need to set different task and resource fetch priorities in NAVIGATING vs. LOADING but on further thought maybe that isn't necessary -- we'd just need to avoid throttling loading tasks in both cases, which means we don't need two distinct states.
Sorry to arrive late. I don't think OnCommitNavigation should ever be called for pushState/replaceState. That will definitely call DidCommitProvisionalLoad, but blink (and not PlzNavigate) should always be driving pushState/replaceState.
I *think* checking PlzNavigate's same document state should suffice to exclude all same-document navigations here.
Great, thanks for confirming. Sounds like we could just add the same-document check to this patch and land it? That'd be super helpful for us to move forward!
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 6:Commit-Queue +1
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 7:Commit-Queue +1
Taiju Tsuiki (Gerrit)
Updated to check IsSameDocument. Though the full fix probably requires another adjustment per http://crbug.com/732738#c18, I think this version is worth doing.
Could you take another look?
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 7:
Patch Set 7:
Updated to check IsSameDocument. Though the full fix probably requires another adjustment per http://crbug.com/732738#c18, I think this version is worth doing.
I agree that exploring the full fix (which should consume this version actually) is worth doing, while we might want to land this sooner to see the impact.
Could you take another look?
(still lgtm)
Sami Kyöstilä (Gerrit)
Sami Kyöstilä posted comments on this change.
Patch set 7:Code-Review +1
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 7:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot merged this change.
Notify RendererScheduler of starting LOADING phase on OnCommitNavigation
Blink Scheduler has been considering that the renderer is in the loading
phase after OnNavigate or DidCommitProvisionalLoad event. However on the
PlzNavigate case, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds OnCommitNavigation as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: I3a9d14d8bb6ec35f9cb56b5dc567afa8f65f2e87
Reviewed-on: https://chromium-review.googlesource.com/542717
Reviewed-by: Sami Kyöstilä <skyo...@chromium.org>
Reviewed-by: Camille Lamy <cl...@chromium.org>
Reviewed-by: Kinuko Yasuda <kin...@chromium.org>
Reviewed-by: Yutaka Hirano <yhi...@chromium.org>
Commit-Queue: Taiju Tsuiki <tz...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482161}
---
M content/renderer/render_frame_impl.cc
1 file changed, 7 insertions(+), 0 deletions(-)
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index d8b13a9..4f762c2 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -5126,6 +5126,13 @@
const CommonNavigationParams& common_params,
const RequestNavigationParams& request_params) {
CHECK(IsBrowserSideNavigationEnabled());
+
+ RenderThreadImpl* render_thread_impl = RenderThreadImpl::current();
+ // Can be NULL in tests.
+ if (render_thread_impl &&
+ !FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type))
+ render_thread_impl->GetRendererScheduler()->OnNavigate();
+
// This will override the url requested by the WebURLLoader, as well as
// provide it with the response to the request.
std::unique_ptr<StreamOverrideParameters> stream_override(
To view, visit change 542717. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 1:Commit-Queue +1
To view, visit change 547377. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki uploaded patch set #2 to this change.
Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad
Blink Scheduler has been considering the renderer is the loading phase
after OnNavigate, DidCommitProvisionalLoad event. However on the renderer
initiated navigation, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds DidStartProvisionalLoad as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: Ic04331e79eefa450da41cf2e1332583a1224880b
---
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/platform/WebFrameScheduler.h
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h
6 files changed, 19 insertions(+), 4 deletions(-)
To view, visit change 547377. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 2:
PTAL
Taiju Tsuiki (Gerrit)
Taiju Tsuiki would like Yutaka Hirano and Sami Kyöstilä to review this change.
Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad
Blink Scheduler has been considering the renderer is the loading phase
after OnNavigate, DidCommitProvisionalLoad event. However on the renderer
initiated navigation, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds DidStartProvisionalLoad as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: Ic04331e79eefa450da41cf2e1332583a1224880b
---
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/platform/WebFrameScheduler.h
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h
6 files changed, 19 insertions(+), 4 deletions(-)
To view, visit change 547377. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
This is PS2 of the previous CL: https://chromium-review.googlesource.com/c/542717/.
The previous one adjust the scheduler phase on a browser initiated navigation case, and this one covers renderer initiated navigations.
Yutaka Hirano (Gerrit)
Yutaka Hirano posted comments on this change.
Patch set 2:Code-Review +1
Sami Kyöstilä (Gerrit)
Sami Kyöstilä posted comments on this change.
Patch set 2:Code-Review +1
Thanks!
(1 comment)
File third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc:
Patch Set #2, Line 1719:
DidStartCommitProvisionalLoadRemove "Commit"
To view, visit change 547377. To unsubscribe, visit settings.
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 3:
Set Ready For Review
Taiju Tsuiki (Gerrit)
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 3:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot posted comments on this change.
Patch set 3:
CQ is trying da patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"typo fix" https://chromium-review.googlesource.com/c/547377/3
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/547377/3
Bot data: {"action": "start", "triggered_at": "2017-06-29T01:54:16.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "7a3b758675b999f5c5d57b507f43613ddae5c18a"}
Commit Bot (Gerrit)
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_presubmit/builds/476993)
Bot data: {"action": "cancel", "triggered_at": "2017-06-29T01:54:16.0Z", "cq_cfg_revision": "1aa54ba037e9058c2752e0a0d871aeabfcba5974", "revision": "7a3b758675b999f5c5d57b507f43613ddae5c18a"}
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 3:
Adding Kinuko-san. PTAL to platform/WebFrameScheduler.h as platform/ owner?
Kinuko Yasuda (Gerrit)
Kinuko Yasuda posted comments on this change.
Patch set 3:Code-Review +1
Taiju Tsuiki (Gerrit)
Taiju Tsuiki posted comments on this change.
Patch set 3:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot merged this change.
Notify RendererScheduler of starting LOADING phase on DidStartProvisionalLoad
Blink Scheduler has been considering the renderer is the loading phase
after OnNavigate, DidCommitProvisionalLoad event. However on the renderer
initiated navigation, OnNavigate doesn't happen and DidCommitProvisionalLoad
happens after the main resource load, so the renderer is not in the loading
phase while a main resource request is in-flight. That may cause a throttled
loading task queue, and delay the main resource load.
This CL adds DidStartProvisionalLoad as a trigger of the loading phase,
so that the main resource is always loaded in an unthrottled task queue.
Bug: 732738
Change-Id: Ic04331e79eefa450da41cf2e1332583a1224880b
Reviewed-on: https://chromium-review.googlesource.com/547377
Reviewed-by: Kinuko Yasuda <kin...@chromium.org>
Reviewed-by: Yutaka Hirano <yhi...@chromium.org>
Reviewed-by: Sami Kyöstilä <skyo...@chromium.org>
Commit-Queue: Taiju Tsuiki <tz...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483295}
---
M third_party/WebKit/Source/core/loader/FrameLoader.cpp
M third_party/WebKit/Source/platform/WebFrameScheduler.h
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/renderer_scheduler_impl.h
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.cc
M third_party/WebKit/Source/platform/scheduler/renderer/web_frame_scheduler_impl.h
6 files changed, 19 insertions(+), 4 deletions(-)