WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
74605
[GTK] Simplify loader client WebKit2 GTK+ API
https://bugs.webkit.org/show_bug.cgi?id=74605
Summary
[GTK] Simplify loader client WebKit2 GTK+ API
Carlos Garcia Campos
Reported
2011-12-15 06:37:24 PST
The initial idea of the loader client object exposed in the API was mainly to reduce the amount of signals of WebView. During the webkitgtk hackfest we were discussing the current API and agreed on moving to simpler API adding just two signals to WebView and making the current loader client private.
Attachments
Patch
(62.06 KB, patch)
2011-12-15 06:53 PST
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Updated patch
(61.04 KB, patch)
2011-12-26 02:12 PST
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
I forgot to include Tools/ChangeLog in previous patch
(61.59 KB, patch)
2011-12-26 02:15 PST
,
Carlos Garcia Campos
gustavo
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2011-12-15 06:53:08 PST
Created
attachment 119424
[details]
Patch The code is simpler and the API easier to use. Now we also have only one condition for load finish. The patch doesn't include previous status in load-status-changed callback as agreed during the hackfest, because Martin has some doubts about it, it's better to add it later if we finally decide it's useful than remve it.
WebKit Review Bot
Comment 2
2011-12-15 06:55:39 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Martin Robinson
Comment 3
2011-12-15 08:29:43 PST
Comment on
attachment 119424
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=119424&action=review
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:316 > + * @status: the #WebKitLoadStatus
I prefer @event: the #WebKitLoadEvent here. See below.
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:71 > +} WebKitLoadStatus;
So I think this should be renamed to WebKitLoadEvent since WEBKIT_LOAD_REDIRECTED is not really a status, but something that happened to us.
Carlos Garcia Campos
Comment 4
2011-12-15 10:47:26 PST
(In reply to
comment #3
)
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:71 > > +} WebKitLoadStatus; > > So I think this should be renamed to WebKitLoadEvent since WEBKIT_LOAD_REDIRECTED is not really a status, but something that happened to us.
hmm, good point!
Martin Robinson
Comment 5
2011-12-15 11:10:11 PST
(In reply to
comment #4
)
> (In reply to
comment #3
) > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:71 > > > +} WebKitLoadStatus; > > > > So I think this should be renamed to WebKitLoadEvent since WEBKIT_LOAD_REDIRECTED is not really a status, but something that happened to us. > > hmm, good point!
I guess we can go two ways here: try to make the enum represent a status or an events For instance if we used events, they enum values would be something like: WEBKIT_LOAD_STARTED_PROVISIONAL WEBKIT_LOAD_REDIRECTED WEBKIT_LOAD_COMMITTED WEBKIT_LOAD_FINISHED and the argument names would be something like previous_load_event instead of previous_load_status. If we think of them as states we would have: WEBKIT_LOAD_PROVISIONAL WEBKIT_LOAD_REDIRECTING WEBKIT_LOAD_COMMITTED WEBKIT_LOAD_FINISHED Personally, I prefer the 'event' structure because that's the interface that the C API gives us and I think it more closely matches what's happening within WebCore.
Gustavo Noronha (kov)
Comment 6
2011-12-16 06:22:15 PST
That's a great approach, Martin, I agree with you!
Carlos Garcia Campos
Comment 7
2011-12-26 02:12:18 PST
Created
attachment 120538
[details]
Updated patch Renamed WebKitLoadStatus to WebKitLoadEvent. I also renamed load-status-changed signal as load-changed to avoid the word 'status' and for consistency with load-failed.
Carlos Garcia Campos
Comment 8
2011-12-26 02:15:05 PST
Created
attachment 120539
[details]
I forgot to include Tools/ChangeLog in previous patch
Gustavo Noronha (kov)
Comment 9
2012-01-03 06:55:23 PST
Comment on
attachment 120539
[details]
I forgot to include Tools/ChangeLog in previous patch View in context:
https://bugs.webkit.org/attachment.cgi?id=120539&action=review
OK, looks good to me
> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:57 > + * fail for transport issues such as not being able to
s/for/due to/ sounds better I think
Carlos Garcia Campos
Comment 10
2012-01-03 08:05:21 PST
Committed
r103938
: <
http://trac.webkit.org/changeset/103938
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug