[webkit-dev] Notifying PageClient when page transition has begun

[Brady Eidson]

On Jul 25, 2013, at 11:28 AM, Thiago Lacerda <thiago.lacerda at openbossa.org> wrote:

> 
> 
> 
> On Thu, Jul 25, 2013 at 2:34 PM, Brady Eidson <beidson at apple.com> wrote:
> 
> On Jul 25, 2013, at 10:10 AM, Thiago Lacerda <thiago.lacerda at openbossa.org> wrote:
> 
>> Hi Brady
>> 
>> 
>> On Thu, Jul 25, 2013 at 1:48 PM, Brady Eidson <beidson at apple.com> wrote:
>> 
>> On Jul 25, 2013, at 9:08 AM, Thiago Lacerda <thiago.lacerda at openbossa.org> wrote:
>> 
>>> Hi,
>>> 
>>> We in the Nix port team (which is going upstream), are facing an issue that we need our WebView to know when the page transition has begun.
>>> There was callback, in PageClient, that was making a similar job, called didCommitLoadForMainFrame, but it was removed recently.
>> 
>> If by “PageClient” you mean the WKPageLoaderClient in WKPage.h, then didCommitLoadForFrame is there.
>> 
>> I don’t even recall there ever being a didCommitLoadForMainFrame, but didCommitLoadForFrame combined with a quick call to WKPageGetMainFrame will do the exact same thing.
>> 
>> By PageClient I mean the one in Source/WebKit2/UIProcess/PageClient.h (which WebView inherits from) 
> 
> I see.  I misunderstood and thought you were talking about the API layer.
> 
> PageClient is largely a platform abstraction and not all platforms inherit a WebView from it.  And since it’s a platform abstraction, I can’t tell you how your platform should behave with it.
> 
> But that’s probably neither here-nor-there…
> 
>>> WebPage does have a callback that tells this (didStartPageTransition), but this is not forwarded to UIProcess, which is kind of weird, since its opposite (didCompletePageTransition) is fired to PageClient when a page has completed its transition.
>> 
>> didStartPageTransition is about starting a provisional load, and didCompletePageTransition is *not* about committing a provisional load.  Additionally, neither of these are actually sent to any API clients;  They are internal to WebPage.
>> 
>> If you think didCommitLoadForFrame() for the main frame was what you wanted, then both of these are irrelevant anyways.
>> 
>> didCompletePageTransition is forwarded to the C API client.
>> In WebPage it calls WebPageProxy::pageTransitionViewportReady -> WebView(inheriting PageClient)::pageTransitionViewportReady -> WebViewClient::didCompletePageTransition
> 
> 
> This is not true in general for the WK2 project.  Cross platform WK2 API is in terms of the WK* objects and their clients.  What you’re describing here is something very platform specific to your platform’s WebView implementation.
> 
> Please take a look at WKViewClient code, you will see that there is a didCompletePageTransition callback

~: grep -r WKViewClient .
...
./UIProcess/API/C/CoordinatedGraphics/WKView.h:struct WKViewClient

That’s not cross platform API, so it’s not a surprise I hadn’t heard of it and that a search of the .xcodeproj didn’t find it.

> Anyways, in your original message, you mentioned that you wanted to know didCommitLoadForFrame for the main frame, and you intended to pipe didStartPageTransition through to the UIProcess to approximate that.
> 
> didStartPageTransition is more about tiled backing stores being set up properly and not about loading transitions.  It’s not remotely the same thing as didCommitLoadForFrame was called for the main frame.
> 
> Hum... maybe I have misunderstood the concepts here, by looking at the name (didPageStartTransition) I though it was totally related with page transitions.

Look at the implementation and the fact that most related code is wrapped by "#if USE(TILED_BACKING_STORE)”

Also, you’re using the phrase “page transitions” which is not a loader concept, yet your original message was asking about a loader concept.

> Despite of that, I think we have an inconsistent API here, which tells when the transition is over but not when it has started.

This is not general cross platform API, but also, the general cross platform API *DOES* have the concept of some callbacks being WebProcess-only while others go to both Web and UI processes.  It’s not an inherent inconsistency.

There’s a reason to not do it, which is to not pay the IPC messaging cost.

> That is the main reason I think it would be good to put that callback in WKViewClient as well.

Noting that you’ve been focusing more on what is and is not API instead of answering this point, I will bring this up one more time:

Are you interested in using didCommitLoadForFrame for the main frame, as you originally asked for?

If so, you already have what you need.

If you do understand what this specific call actually means and you actually need it in the UI Process, I would ask that the messaging be wrapped with "#if USE(TILED_BACKING_STORE)” so the non-main ports don’t pay the cost.

If you’re simply interested in adding new API because you think it makes things more consistent, even if it’s not what you need, then I would say that’s a terrible idea.

Thanks,
~Brady

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130725/6ccf01da/attachment-0001.html>