Bug 141563

Summary: [EFL] Implement WebView::rootViewToScreen.
Product: WebKit Reporter: Julien Isorce <j.isorce>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: andersca, benjamin, lucas.de.marchi, mcatanzaro, obzhirov
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
[EFL] Implement WebView::rootViewToScreen. achristensen: review-

Description Julien Isorce 2015-02-13 07:21:16 PST
Currently Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp::rootViewToScreen(const IntRect& rect) is not implemented.
Comment 1 Julien Isorce 2015-02-13 07:33:09 PST
Created attachment 246523 [details]
[EFL] Implement WebView::rootViewToScreen.
Comment 2 Gyuyoung Kim 2015-02-16 16:42:03 PST
Comment on attachment 246523 [details]
[EFL] Implement WebView::rootViewToScreen.

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

I don't object to implement new function. However I think you need to explain why this function is needed for us.

> Source/WebKit2/ChangeLog:8
> +        All of these are just glue except EwkView::rootViewToScreen.

Could you explain what is a role of rootViewToScreen ?

> Source/WebKit2/UIProcess/WebPageProxy.h:1104
>      void screenToRootView(const WebCore::IntPoint& screenPoint, WebCore::IntPoint& windowPoint);

If you need to use rootViewToScreen(), don't we need to support screenToRootView() as well ?

> Source/WebKit2/UIProcess/WebPageProxy.h:-1104
> -    void rootViewToScreen(const WebCore::IntRect& viewRect, WebCore::IntRect& result);

This move should be approved by WK2 owner.
Comment 3 Julien Isorce 2015-02-17 05:15:28 PST
(In reply to comment #2)
> Comment on attachment 246523 [details]
> [EFL] Implement WebView::rootViewToScreen.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=246523&action=review
> 
> I don't object to implement new function. However I think you need to
> explain why this function is needed for us.

First thx for the initial review.

This is not a really new function. This is an existing function that is currently implemented with "NotImplemented" for coordinates graphics: WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp::WebView::rootViewToScreen.

I am not sure of the history but I suspect it was supported with WebKit1 but it has not been fully ported to WebKit2.

Also in Source/WebCore/platform/HostWindow.h there is
virtual IntRect rootViewToScreen(const IntRect&) const = 0;

Actually it is implemented for GTK already:
WebKit2/UIProcess/API/gtk/PageClientImpl.cpp::PageClientImpl::rootViewToScreen

Also the following code already exists in WebKit2/WebProcess/WebPage/WebPage.pp:

IntPoint WebPage::screenToRootView(const IntPoint& point)
{
    IntPoint windowPoint;
    sendSync(Messages::WebPageProxy::ScreenToRootView(point), Messages::WebPageProxy::ScreenToRootView::Reply(windowPoint));
    return windowPoint;
}
    
IntRect WebPage::rootViewToScreen(const IntRect& rect)
{
    IntRect screenRect;
    sendSync(Messages::WebPageProxy::RootViewToScreen(rect), Messages::WebPageProxy::RootViewToScreen::Reply(screenRect));
    return screenRect;
}


> 
> > Source/WebKit2/ChangeLog:8
> > +        All of these are just glue except EwkView::rootViewToScreen.
> 
> Could you explain what is a role of rootViewToScreen ?

The general idea is to be able to know the webkit view position in the screen within the webprocess. The webprocess needs some informations about the root view that only the UI process knows.

> 
> > Source/WebKit2/UIProcess/WebPageProxy.h:1104
> >      void screenToRootView(const WebCore::IntPoint& screenPoint, WebCore::IntPoint& windowPoint);
> 
> If you need to use rootViewToScreen(), don't we need to support
> screenToRootView() as well ?

Yes indeed I should add screenToRootView as well.

> 
> > Source/WebKit2/UIProcess/WebPageProxy.h:-1104
> > -    void rootViewToScreen(const WebCore::IntRect& viewRect, WebCore::IntRect& result);
> 
> This move should be approved by WK2 owner.

All right, I'll add some in CC
Comment 4 Alex Christensen 2017-03-03 08:31:50 PST
Comment on attachment 246523 [details]
[EFL] Implement WebView::rootViewToScreen.

EFL port was removed.  This patch is no longer necessary in trunk.
Comment 5 Michael Catanzaro 2017-03-11 10:49:15 PST
Closing this bug because the EFL port has been removed from trunk.

If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.