Bug 108920 - [Qt][WK2] Fold QtWebPageFindClient into QQuickWebViewPrivate
Summary: [Qt][WK2] Fold QtWebPageFindClient into QQuickWebViewPrivate
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Hausmann
URL:
Keywords:
Depends on:
Blocks: 108471
  Show dependency treegraph
 
Reported: 2013-02-05 00:47 PST by Simon Hausmann
Modified: 2013-02-07 00:39 PST (History)
6 users (show)

See Also:


Attachments
Patch (10.06 KB, patch)
2013-02-05 00:50 PST, Simon Hausmann
jturcotte: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hausmann 2013-02-05 00:47:40 PST
[Qt][WK2] Fold QtWebPageFindClient into QQuickWebViewPrivate
Comment 1 Simon Hausmann 2013-02-05 00:50:11 PST
Created attachment 186567 [details]
Patch
Comment 2 Jocelyn Turcotte 2013-02-05 06:15:23 PST
Comment on attachment 186567 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:327
> +    {
> +        WKPageFindClient findClient;
> +        memset(&findClient, 0, sizeof(WKPageFindClient));
> +        findClient.version = kWKPageFindClientCurrentVersion;
> +        findClient.clientInfo = this;
> +        findClient.didFindString = didFindString;
> +        findClient.didFailToFindString = didFailToFindString;
> +        WKPageSetPageFindClient(toAPI(webPageProxy.get()), &findClient);
> +    }

If the intent is to do so with most C API clients, it would be cleaner move all their initialization in a separate method.
We can also do this once we have more of them here.
r=me either way.
Comment 3 Simon Hausmann 2013-02-05 07:24:52 PST
This still needs a sign-off from a WK2 owner. Benjamin, can you help us with this? :)
Comment 4 Benjamin Poulain 2013-02-05 17:29:24 PST
Comment on attachment 186567 [details]
Patch

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

Okay with me, feel free to land.

Isn't that exposing a bit too much in case you want to make QQuickWebView publicly available from C++?

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:2082
> +    didFindString(page, string, 0, clientInfo);

Maybe prefix by QQuickWebViewPrivate:: for clarity?
Comment 5 Simon Hausmann 2013-02-07 00:25:18 PST
(In reply to comment #4)
> (From update of attachment 186567 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186567&action=review
> 
> Okay with me, feel free to land.

Thank you!
 
> Isn't that exposing a bit too much in case you want to make QQuickWebView 
publicly available from C++?

It is becoming less and less likely that we're going to make the QML implementation a public C++ API :)
 
> > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:2082
> > +    didFindString(page, string, 0, clientInfo);
> 
> Maybe prefix by QQuickWebViewPrivate:: for clarity?

Good idea.
Comment 6 Simon Hausmann 2013-02-07 00:39:57 PST
Committed r142073: <http://trac.webkit.org/changeset/142073>