Bug 58024 – [GTK] Implement page loader client in MiniBrowser

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

It allows us to update the window title and url entry.

[Carlos Garcia Campos]

Created attachment 88602 [details]
Patch

It adds a new class BrowserWindow that implements the page loader client and will make supporting multiple windows easier. This patch applies on top of patch attached to bug #57944

[Martin Robinson]

Comment on attachment 88602 [details]
Patch

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

Nice.

> Tools/MiniBrowser/gtk/BrowserWindow.c:372
> +    WKPageLoaderClient loadClient = {
> +        0,       /* version */
> +        window, /* clientInfo */
> +        didStartProvisionalLoadForFrame,
> +        didReceiveServerRedirectForProvisionalLoadForFrame,
> +        didFailProvisionalLoadWithErrorForFrame,
> +        didCommitLoadForFrame,
> +        didFinishDocumentLoadForFrame,
> +        didFinishLoadForFrame,
> +        didFailLoadWithErrorForFrame,
> +        0, /* didSameDocumentNavigationForFrame */
> +        didReceiveTitleForFrame,
> +        didFirstLayoutForFrame,
> +        didFirstVisuallyNonEmptyLayoutForFrame,
> +        didRemoveFrameFromHierarchy,
> +        0, /* didDisplayInsecureContentForFrame */
> +        0, /* didRunInsecureContentForFrame */
> +        0, /* canAuthenticateAgainstProtectionSpaceInFrame */
> +        0, /* didReceiveAuthenticationChallengeInFrame */
> +        didStartProgress,
> +        didChangeProgress,
> +        didFinishProgress,
> +        didBecomeUnresponsive,
> +        didBecomeResponsive,
> +        0,  /* processDidCrash */
> +        0,  /* didChangeBackForwardList */
> +        0   /* shouldGoToBackForwardListItem */
> +    };
> +    WKPageSetPageLoaderClient(WKViewGetPage(window->webView), &loadClient);

Do you mind lining up the comments on these lines?

> Tools/MiniBrowser/gtk/BrowserWindow.c:376
> +GtkWidget* browserWindowNew(WKViewRef view)

Minor bikeshed. Since BrowserWindow is a GObject, I think the public methods should have g_object_naming_style.

[Carlos Garcia Campos]

(In reply to comment #2)
> (From update of attachment 88602 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88602&action=review
> 
> Nice.
> 
> > Tools/MiniBrowser/gtk/BrowserWindow.c:372
> > +    WKPageLoaderClient loadClient = {
> > +        0,       /* version */
> > +        window, /* clientInfo */
> > +        didStartProvisionalLoadForFrame,
> > +        didReceiveServerRedirectForProvisionalLoadForFrame,
> > +        didFailProvisionalLoadWithErrorForFrame,
> > +        didCommitLoadForFrame,
> > +        didFinishDocumentLoadForFrame,
> > +        didFinishLoadForFrame,
> > +        didFailLoadWithErrorForFrame,
> > +        0, /* didSameDocumentNavigationForFrame */
> > +        didReceiveTitleForFrame,
> > +        didFirstLayoutForFrame,
> > +        didFirstVisuallyNonEmptyLayoutForFrame,
> > +        didRemoveFrameFromHierarchy,
> > +        0, /* didDisplayInsecureContentForFrame */
> > +        0, /* didRunInsecureContentForFrame */
> > +        0, /* canAuthenticateAgainstProtectionSpaceInFrame */
> > +        0, /* didReceiveAuthenticationChallengeInFrame */
> > +        didStartProgress,
> > +        didChangeProgress,
> > +        didFinishProgress,
> > +        didBecomeUnresponsive,
> > +        didBecomeResponsive,
> > +        0,  /* processDidCrash */
> > +        0,  /* didChangeBackForwardList */
> > +        0   /* shouldGoToBackForwardListItem */
> > +    };
> > +    WKPageSetPageLoaderClient(WKViewGetPage(window->webView), &loadClient);
> 
> Do you mind lining up the comments on these lines?

Sure.

> > Tools/MiniBrowser/gtk/BrowserWindow.c:376
> > +GtkWidget* browserWindowNew(WKViewRef view)
> 
> Minor bikeshed. Since BrowserWindow is a GObject, I think the public methods should have g_object_naming_style.

Well, this is not exactly public API, it's private to minibrowser, but anyway, I prefer gnome style so I'll happily change it. 

Note that this patch depends on patch attached to bug #57944.

[Carlos Garcia Campos]

Committed r85311: <http://trac.webkit.org/changeset/85311>