Bug 117116 – [WK2][CoordinatedGraphics]: Use a properly initialized WebPage when creating a PageClient

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sergio Correia (qrwteyrutiyoup)]

[WK2][CG]: Use a properly initialized WebPage when creating a PageClient

[Sergio Correia (qrwteyrutiyoup)]

Created attachment 203501 [details]
Patch

[Darin Adler]

Comment on attachment 203501 [details]
Patch

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

> Source/WebKit2/UIProcess/CoordinatedGraphics/WebView.cpp:53
> +    m_page = context->createWebPage(this, pageGroup);

Needs a comment so someone doesn’t just move it back. Such as:

    // Need to call createWebPage after other data members, specifically m_visible, are initialized.

[Sergio Correia (qrwteyrutiyoup)]

Created attachment 203505 [details]
Patch

Added comment as requested by Darin.

[Tim Horton]

Comment on attachment 203505 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [WK2][CG]: Use a properly initialized WebPage when creating a PageClient

Oh, maybe we can use something other than [CG] in titles for Coordinated Graphics, since we already use that for CoreGraphics?

[Sergio Correia (qrwteyrutiyoup)]

(In reply to comment #4)
> (From update of attachment 203505 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=203505&action=review
> 
> > Source/WebKit2/ChangeLog:3
> > +        [WK2][CG]: Use a properly initialized WebPage when creating a PageClient
> 
> Oh, maybe we can use something other than [CG] in titles for Coordinated Graphics, since we already use that for CoreGraphics?

My bad, I wasn't aware CG was already used for CoreGraphics. 

Would you like me to update the bug title (and the commit message/changleog) to use CoordinatedGraphics instead?

[Tim Horton]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 203505 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=203505&action=review
> > 
> > > Source/WebKit2/ChangeLog:3
> > > +        [WK2][CG]: Use a properly initialized WebPage when creating a PageClient
> > 
> > Oh, maybe we can use something other than [CG] in titles for Coordinated Graphics, since we already use that for CoreGraphics?
> 
> My bad, I wasn't aware CG was already used for CoreGraphics. 
> 
> Would you like me to update the bug title (and the commit message/changleog) to use CoordinatedGraphics instead?

Sure :)

[Sergio Correia (qrwteyrutiyoup)]

Created attachment 203509 [details]
Patch

Changed [CG] tag to [CoordinatedGraphics], as requested by Tim, as the former refers to CoreGraphics.

[Tim Horton]

Comment on attachment 203509 [details]
Patch

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

> Source/WebKit2/ChangeLog:42
> +        Here's the relevant valgrind trace for reference:
> +
> +        Conditional jump or move depends on uninitialised value(s)
> +            at 0x65A35A4: WebKit::WebPageProxy::WebPageProxy(WebKit::PageClient*,
> +                          WTF::PassRefPtr<WebKit::WebProcessProxy>,
> +                          WebKit::WebPageGroup*, unsigned long) (WebPageProxy.cpp:322)
> +            by 0x65A2BA2: WebKit::WebPageProxy::create(WebKit::PageClient*,
> +                          WTF::PassRefPtr<WebKit::WebProcessProxy>,
> +                          WebKit::WebPageGroup*, unsigned long) (WebPageProxy.cpp:233)
> +            by 0x65E94BB: WebKit::WebProcessProxy::createWebPage(WebKit::PageClient*,
> +                          WebKit::WebContext*, WebKit::WebPageGroup*)
> +                          (WebProcessProxy.cpp:172)
> +            by 0x6570957: WebKit::WebContext::createWebPage(WebKit::PageClient*,
> +                          WebKit::WebPageGroup*, WebKit::WebPageProxy*)
> +                          (WebContext.cpp:735)
> +            by 0x67673E3: WebKit::WebView::WebView(WebKit::WebContext*,
> +                          WebKit::WebPageGroup*) (WebView.cpp:52)
> +            by 0x6775F18: WebKit::WebViewEfl::WebViewEfl(WebKit::WebContext*,
> +                          WebKit::WebPageGroup*) (WebViewEfl.cpp:54)
> +            by 0x6775EB4: WebKit::WebView::create(WebKit::WebContext*,
> +                          WebKit::WebPageGroup*) (WebViewEfl.cpp:49)
> +            by 0x673E13D: WKViewCreate (WKView.cpp:33)
> +            by 0x6763ECE: EWKViewCreate (ewk_view.cpp:92)

This is a bit excessive for the changelog, I think, but it's fine.

[Sergio Correia (qrwteyrutiyoup)]

(In reply to comment #8)
> (From update of attachment 203509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=203509&action=review
> 
[snip]
> 
> This is a bit excessive for the changelog, I think, but it's fine.

Alright, I will keep that in mind :)
Thanks!

[WebKit Commit Bot]

Comment on attachment 203509 [details]
Patch

Clearing flags on attachment: 203509

Committed r151081: <http://trac.webkit.org/changeset/151081>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.