[webkit-reviews] review granted: [Bug 73150] [Qt] [WK2] Remove WebContext related code from QtWebPageProxy : [Attachment 116643] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Caio Marcelo de
Oliveira Filho <cmarcelo at webkit.org>'s request for review:
Bug 73150: [Qt] [WK2] Remove WebContext related code from QtWebPageProxy
https://bugs.webkit.org/show_bug.cgi?id=73150

Attachment 116643: Patch
https://bugs.webkit.org/attachment.cgi?id=116643&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=116643&action=review


> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:49
> +// Used only by WebKitTestRunner. Not calling initialize() so we don't
register any clients.

It avoids calling initialize(), so that we don't ...

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:64
> +    defaultContext->initialize();
> +    return defaultContext.release();

I prefer a newline before the return. It makes it easier to spot that it
returns something

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:67
> +PassRefPtr<WebPageProxy> QtWebContext::createWebPage(PageClient* pageClient,
WebPageGroup* webPageGroup)

Personally I would just call the arguments for client and pageGroup. At least
pageGroup should be enough

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:72
> +void QtWebContext::setNavigatorQtObjectEnabled(WebPageProxy* webPageProxy,
bool enabled)

What about setNavigationDotQtJSObjectEnabled...  It is hard to figure out what
NavigationQt is all about

> Source/WebKit2/UIProcess/qt/QtWebContext.cpp:98
> +void QtWebContext::setupContextInjectedBundleClient()

Any reason this is called setup and not initialize?