Bug 97773

Summary: [Qt] Wait for the UI process before re-enabling rendering during page load
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, cmarcelo, hausmann, jturcotte, kenneth, menard, ossy, webkit.review.bot, zoltan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 98045    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch kenneth: review+

Jocelyn Turcotte
Reported 2012-09-27 05:23:11 PDT
[Qt] Wait for the UI process before re-enabling rendering during page load
Attachments
Patch (17.68 KB, patch)
2012-09-27 05:31 PDT, Jocelyn Turcotte 		no flags
DetailsFormatted DiffDiff
Patch (17.87 KB, patch)
2012-09-28 04:52 PDT, Jocelyn Turcotte 		kenneth: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jocelyn Turcotte
Comment 1 2012-09-27 05:31:25 PDT
Created attachment 165979 [details] Patch
Kenneth Rohde Christiansen
Comment 2 2012-09-27 07:53:48 PDT
Comment on attachment 165979 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=165979&action=review > Source/WebKit2/UIProcess/qt/WebPageProxyQt.cpp:127 > +void WebPageProxy::initialViewportReady() viewportReadyForNewPage ? I think that is more clear.
Jocelyn Turcotte
Comment 3 2012-09-27 08:48:39 PDT
(In reply to comment #2) > (From update of attachment 165979 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=165979&action=review > > > Source/WebKit2/UIProcess/qt/WebPageProxyQt.cpp:127 > > +void WebPageProxy::initialViewportReady() > > viewportReadyForNewPage ? I think that is more clear. I tried to find a good name for that one for a while but it's not easy. What this signal means is essentially that the web process sent all page load related viewport messages and wants the ui process to confirm that it handled them and sent back the resulting visible rect. The name should also reflect that the web process will be locked until the commitInitialViewport message is sent back. I'm not sure about NewPage, it sounds like it applies when a new WebPage is created. Any other idea?
Kenneth Rohde Christiansen
Comment 4 2012-09-27 08:53:46 PDT
transitionToCommittedForNewPage is used elsewhere
Jocelyn Turcotte
Comment 5 2012-09-27 09:44:29 PDT
(In reply to comment #4) > transitionToCommittedForNewPage is used elsewhere "ForNewPage" is used as a contrast for "FromCachedFrame" and makes sense in the context of the frame loader, also because it starts with "transition". But I think that it's ambiguous in WebPage. What about this: viewportReadyForLoadedPage commitViewportForLoadedPage
Kenneth Rohde Christiansen
Comment 6 2012-09-27 10:11:54 PDT
(In reply to comment #5) > (In reply to comment #4) > > transitionToCommittedForNewPage is used elsewhere > > > "ForNewPage" is used as a contrast for "FromCachedFrame" and makes sense in the context of the frame loader, also because it starts with "transition". But I think that it's ambiguous in WebPage. > > What about this: > viewportReadyForLoadedPage > commitViewportForLoadedPage does it have to be fully loaded? Maybe just newViewportReady? I guess it will be called if someone changes the viewport meta tag programmatically?
Jocelyn Turcotte
Comment 7 2012-09-27 10:18:05 PDT
> does it have to be fully loaded? > > Maybe just newViewportReady? I guess it will be called if someone changes the viewport meta tag programmatically? initialViewportReady is sent by didCompletePageTransition which is called either by dispatchDidLayout or frameLoadCompleted, whichever comes first. So it is sent instead of unfreezing the rendering directly after the first layout.
Kenneth Rohde Christiansen
Comment 8 2012-09-27 10:23:27 PDT
viewportReadyForPageTransition ?
Jocelyn Turcotte
Comment 9 2012-09-28 04:20:54 PDT
(In reply to comment #8) > viewportReadyForPageTransition ? commitViewportForPageTransition sounds like it's happening before the transition while it should be at the end. I'd put it that way, what do you think: pageTransitionViewportReady commitPageTransitionViewport /me crosses fingers
Kenneth Rohde Christiansen
Comment 10 2012-09-28 04:29:23 PDT
I thikn that is ok
Jocelyn Turcotte
Comment 11 2012-09-28 04:52:58 PDT
Created attachment 166210 [details] Patch s/initial/pageTransition/
Jocelyn Turcotte
Comment 12 2012-10-01 04:37:21 PDT
Committed r130029: <http://trac.webkit.org/changeset/130029>
Csaba Osztrogonác
Comment 13 2012-10-01 10:00:58 PDT
(In reply to comment #12) > Committed r130029: <http://trac.webkit.org/changeset/130029> It caused a regression - https://bugs.webkit.org/show_bug.cgi?id=98045 Could you check it, please?
Note You need to log in before you can comment on or make changes to this bug.