Bug 58024

Summary: [GTK] Implement page loader client in MiniBrowser
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 57944    
Bug Blocks: 58416    
Attachments:
Description Flags
Patch mrobinson: review+

Description Carlos Garcia Campos 2011-04-07 03:10:49 PDT
It allows us to update the window title and url entry.
Comment 1 Carlos Garcia Campos 2011-04-07 03:17:09 PDT
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
Comment 2 Martin Robinson 2011-04-13 08:04:51 PDT
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.
Comment 3 Carlos Garcia Campos 2011-04-13 08:11:55 PDT
(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.
Comment 4 Carlos Garcia Campos 2011-04-29 02:33:09 PDT
Committed r85311: <http://trac.webkit.org/changeset/85311>