Bug 46165 – Focused frame information needed in Chrome and ChromeClient

• Component: WebCore Misc.
• Status: RESOLVED DUPLICATE

[Juha Savolainen]

Chrome and ChromeClient should support method which is called when focused frame has changed.
In WebKit2 we need this information when accessing current frame. With this change we can avoid
synchronized call from UIProcess to WebProcess when using frames.

[Juha Savolainen]

Created attachment 68192 [details]
Added a new method to Chrome and ChromeClient which is called when focused frame has changed

[Antonio Gomes]

I think the patch is straightforward and reasonable. Question:

1) Do we need this for WebKit1 too?
2) Should the various ChromeClient in WebKit/ get updated?
3) Should we pass the Focused frame as parameter?
4) Can we test that somehow? even if it is an autotest ... (of course layout test preferred)

[Sam Weinig]

Comment on attachment 68192 [details]
Added a new method to Chrome and ChromeClient which is called when focused frame has changed

View in context: https://bugs.webkit.org/attachment.cgi?id=68192&action=review

> WebCore/page/ChromeClient.h:90
> +        virtual void focusedFrameChanged() {};

I would prefer if this was pure-virtual like most of the others in this file.


Can you also explain the use case a little more about the use case for having this information in the UIProcess. Why is it necessary to know which frame has focus?

[Juha Savolainen]

(In reply to comment #2)
> I think the patch is straightforward and reasonable. Question:
> 
> 1) Do we need this for WebKit1 too?

Well, I think no. This notification was added because of WebKit2 architecture(UIProcess/WebProcess). In WebKit1 it is quite simply to ask focused frame from WebCore and return that to the user immeadiately. Ofcourse this could be also used in WebKit1 but I think  that there isn't such benefit than in WebKit2.

> 2) Should the various ChromeClient in WebKit/ get updated?

My idea is that this only affect to WebKit2-side.

> 3) Should we pass the Focused frame as parameter?

Maybe not. We only need to resolve WebKit2::WebFrame.frameID which is focused. Also a little bit later I will introduce WebPage::currentFrameID() method in WebKit2 which returns ID of focused WebFrame. Because this method dosen't take parameter WebKit::Frame* this could be used allways to resolve focused framedID, even if we don't know which WebKit::Frame is currently activated. In our case, when focused frame has changed and FocusController calls WebChromeClient::focusedFrameChanged() we just call currentFrameID() method and send frameID to UIProcess.

> 4) Can we test that somehow? even if it is an autotest ... (of course layout test preferred)

Cannot answer right now, I need investigate testing a little bit more how this kind of functions is tested earlyer.

[Juha Savolainen]

(In reply to comment #3)
> (From update of attachment 68192 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=68192&action=review
> 
> > WebCore/page/ChromeClient.h:90
> > +        virtual void focusedFrameChanged() {};
> 
> I would prefer if this was pure-virtual like most of the others in this file.

Pure virtual means that it must be implemented in derived class and I thought that we need this method only in WebKit2-side. If this is changed to pure virtual method then it must be added to all ChromeClients. And implementation would be empty.

> Can you also explain the use case a little more about the use case for having this information in the UIProcess. Why is it necessary to know which frame has focus?

Yes, sure.
Main point is that this is needed because I have done QWKFrame API(initial version, just 5 method) to WebKit2 which is similar than QWebFrame API. The rest patches will come later.

I will add a new class, QWKFrame which have eg. frameName-method which returns frame name. 
The use case: user wants to know name of the current frame(=focused frame), currently WebKit2 C-API dosen't support any other than mainframe. If we don't do this notification, then we must do synchronized call from UIProcess to WebProcess everytime when someone wants access to frames. I think we should send frameID to UIProcess when it's changed so it will be there allready.

[Sam Weinig]

(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 68192 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=68192&action=review
> > 
> > > WebCore/page/ChromeClient.h:90
> > > +        virtual void focusedFrameChanged() {};
> > 
> > I would prefer if this was pure-virtual like most of the others in this file.
> 
> Pure virtual means that it must be implemented in derived class and I thought that we need this method only in WebKit2-side. If this is changed to pure virtual method then it must be added to all ChromeClients. And implementation would be empty.

Yes. That is the normal way we do things.

> 
> > Can you also explain the use case a little more about the use case for having this information in the UIProcess. Why is it necessary to know which frame has focus?
> 
> Yes, sure.
> Main point is that this is needed because I have done QWKFrame API(initial version, just 5 method) to WebKit2 which is similar than QWebFrame API. The rest patches will come later.
> 
> I will add a new class, QWKFrame which have eg. frameName-method which returns frame name. 
> The use case: user wants to know name of the current frame(=focused frame), currently WebKit2 C-API dosen't support any other than mainframe. If we don't do this notification, then we must do synchronized call from UIProcess to WebProcess everytime when someone wants access to frames. I think we should send frameID to UIProcess when it's changed so it will be there allready.

I am still not clear on why the user needs to know the name of the focused frame?  What can they do with that information?

[Juha Savolainen]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > (From update of attachment 68192 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=68192&action=review
> > > 
> > > > WebCore/page/ChromeClient.h:90
> > > > +        virtual void focusedFrameChanged() {};
> > > 
> > > I would prefer if this was pure-virtual like most of the others in this file.
> > 
> > Pure virtual means that it must be implemented in derived class and I thought that we need this method only in WebKit2-side. If this is changed to pure virtual method then it must be added to all ChromeClients. And implementation would be empty.
> 
> Yes. That is the normal way we do things.

Ok, if that is rule then it should fix to pure virtual altought I have a little bit different opinion.

> I am still not clear on why the user needs to know the name of the focused frame?  What can they do with that information?

Sorry, my mistake. I meaned focused frame url.

[Juha Savolainen]

Created attachment 68506 [details]
Add focusedFrameChanged-method to ChromeClients

[Antonio Gomes]

Comment on attachment 68506 [details]
Add focusedFrameChanged-method to ChromeClients

It looks sane to me. Sam?

[Sam Weinig]

> 
> > I am still not clear on why the user needs to know the name of the focused frame?  What can they do with that information?
> 
> Sorry, my mistake. I meaned focused frame url.

I am still not sure what the use case is.

[Juha Savolainen]

(In reply to comment #10)
> > 
> > > I am still not clear on why the user needs to know the name of the focused frame?  What can they do with that information?
> > 
> > Sorry, my mistake. I meaned focused frame url.
> 
> I am still not sure what the use case is.

Html-page contains 3 frames(main frame + 2 other) and the user wants to know 
url of any other frame than a mainframe, it's impossible. 
WebKit1: QWebPage->currentFrame()->url()

Html-page contains 3 frames(main frame + 2 other) and the user just wants to
access the current frame, it's impossible. 
WebKit1: QWebPage->currentFrame();

In WebKit2, there is only possibility to operate with the mainframe, 
not focused or any other than mainframe.

[Sam Weinig]

(In reply to comment #11)
> (In reply to comment #10)
> > > 
> > > > I am still not clear on why the user needs to know the name of the focused frame?  What can they do with that information?
> > > 
> > > Sorry, my mistake. I meaned focused frame url.
> > 
> > I am still not sure what the use case is.
> 
> Html-page contains 3 frames(main frame + 2 other) and the user wants to know 
> url of any other frame than a mainframe, it's impossible. 
> WebKit1: QWebPage->currentFrame()->url()
> 
> Html-page contains 3 frames(main frame + 2 other) and the user just wants to
> access the current frame, it's impossible. 
> WebKit1: QWebPage->currentFrame();
> 
> In WebKit2, there is only possibility to operate with the mainframe, 
> not focused or any other than mainframe.

I understand how it can be used, but *why* does a user need to know which frame is focused from the UIProcess.  The model we have been trying to work with is to have almost everything be based on the Page, and have frames act almost as handles to the load state.

[Juha Savolainen]

(In reply to comment #12)

> I understand how it can be used, but *why* does a user need to know which frame is focused from the UIProcess.  The model we have been trying to work with is to have almost everything be based on the Page, and have frames act almost as handles to the load state.

Thank you for comments. This would related to new API for frames, could you say your opinion about that API?

[Kenneth Rohde Christiansen]

Hi Simon, do you know the reason why we originally added the currentFrame method? That seems to be before I joined the project.

[Alexey Proskuryakov]

This never had a use case mentioned, but we ended up needing to know the focused frame in Safari.

*** This bug has been marked as a duplicate of bug 48685 ***