Currently the WebKitWebLoaderClient is structured in such a way that it can only be attached to one WebView. Since C API page client callbacks carry the page as an argument, this restriction doesn't need to exist. This bug tracks allowing sharing clients between WebViews. This more closely matches the WebKit1 delegates that the Mac port uses and the WebKit2 C API.
Created attachment 110469 [details] Patch
Created attachment 110474 [details] Patch also disconnecting new signal handler on fixture destruction
Comment on attachment 110474 [details] Patch also disconnecting new signal handler on fixture destruction View in context: https://bugs.webkit.org/attachment.cgi?id=110474&action=review Patch looks good to me, maybe you can use current test instead of making this patch depend on uncomitted patches, so that it can land as soon as it's reviewed. > Source/WebKit2/UIProcess/API/gtk/WebKitDefines.h:38 > +typedef struct _WebKitWebView WebKitWebView; > +typedef struct _WebKitWebViewClass WebKitWebViewClass; > + > +typedef struct _WebKitWebLoaderClient WebKitWebLoaderClient; > +typedef struct _WebKitWebLoaderClientClass WebKitWebLoaderClientClass; > + This change is unrelated, I prefer typedefs of every instance/class in their own header, or is there a reason to hae all of them here?
Comment on attachment 110474 [details] Patch also disconnecting new signal handler on fixture destruction View in context: https://bugs.webkit.org/attachment.cgi?id=110474&action=review >> Source/WebKit2/UIProcess/API/gtk/WebKitDefines.h:38 >> + > > This change is unrelated, I prefer typedefs of every instance/class in their own header, or is there a reason to hae all of them here? I was unhappy with this change as well, but WebKitWebLoaderClient.h refers to WebKitWebView* and WebKitWebView.h refers to WebKitWebLoaderClient*. If you can think of another way to solve the issue, I'll give it a shot.
Comment on attachment 110474 [details] Patch also disconnecting new signal handler on fixture destruction The patch looks good to me, one suggestion maybe, have a test with a single loader client handling multiple webViews?
Comment on attachment 110474 [details] Patch also disconnecting new signal handler on fixture destruction Seems to be basically just moving code around, so looks good to me (without having much context about this stuff yet!). A couple of small comments: - Seems the estimated progress property could have been moved to the view in a previous patch. Would have made things easier to review? - I agree with Phil that some code testing the new possibility of multiple views per loader client makes sense.
(In reply to comment #6) > (From update of attachment 110474 [details]) > Seems to be basically just moving code around, so looks good to me (without having much context about this stuff yet!). A couple of small comments: > - Seems the estimated progress property could have been moved to the view in a previous patch. Would have made things easier to review? > - I agree with Phil that some code testing the new possibility of multiple views per loader client makes sense. Thanks for the review. I landed this patch with a new unit test as described.
Committed r97920: <http://trac.webkit.org/changeset/97920>