Summary: | [GTK] Move the API to get TLS certificate from WebKitURIResponse to WebKitWebView | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | andersca, danw, gustavo, gyuyoung.kim, mrobinson, rakuco, sam, webkit.review.bot, xan.lopez | ||||
Priority: | P2 | Keywords: | Gtk | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 109287 | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2013-02-19 01:15:42 PST
Created attachment 189019 [details]
Patch
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 Comment on attachment 189019 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189019&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2907 > + * from the server, or %FALSE otherwise. I believe you can drop this 'or', it's made redundant by 'otherwise'. (In reply to comment #3) > (From update of attachment 189019 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189019&action=review Thanks for the review, but remember this has to be r+'ed by an owner. > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2907 > > + * from the server, or %FALSE otherwise. > > I believe you can drop this 'or', it's made redundant by 'otherwise'. I think martin insisted in adding that 'or', but I'm not sure, I'm not English expert :-P Comment on attachment 189019 [details]
Patch
I just remembered/realized this needs to be looked at by an OWNER, but it looks good to me from a purely GTK+ perspective.
Comment on attachment 189019 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189019&action=review Looks good to me as well. This will need an owner's review. > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2896 > + * @certificate: (out) (transfer none): return location for a #GTlsCertificate > + * @errors: (out): return location for a #GTlsCertificateFlags the verification status of @certificate Is there an equivalent to "allow none" for out parameters or is it just assumed? >>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2907 >>> + * from the server, or %FALSE otherwise. >> >> I believe you can drop this 'or', it's made redundant by 'otherwise'. > > I think martin insisted in adding that 'or', but I'm not sure, I'm not English expert :-P I think the 'or' is fine. > Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:51 > GTlsCertificate* certificate = 0; > - webkit_uri_response_get_https_status(response, &certificate, &m_tlsErrors); > + webkit_web_view_get_tls_info(m_webView, &certificate, &m_tlsErrors); > m_certificate = certificate; Why not just use &m_certificate here? (In reply to comment #6) > (From update of attachment 189019 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=189019&action=review > > Looks good to me as well. This will need an owner's review. > > > Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2896 > > + * @certificate: (out) (transfer none): return location for a #GTlsCertificate > > + * @errors: (out): return location for a #GTlsCertificateFlags the verification status of @certificate > > Is there an equivalent to "allow none" for out parameters or is it just assumed? It's assumed. > >>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:2907 > >>> + * from the server, or %FALSE otherwise. > >> > >> I believe you can drop this 'or', it's made redundant by 'otherwise'. > > > > I think martin insisted in adding that 'or', but I'm not sure, I'm not English expert :-P > > I think the 'or' is fine. > > > Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:51 > > GTlsCertificate* certificate = 0; > > - webkit_uri_response_get_https_status(response, &certificate, &m_tlsErrors); > > + webkit_web_view_get_tls_info(m_webView, &certificate, &m_tlsErrors); > > m_certificate = certificate; > > Why not just use &m_certificate here? Is that possible with a GRefPtr? Comment on attachment 189019 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189019&action=review >> Source/WebKit2/UIProcess/API/gtk/tests/TestSSL.cpp:51 >> m_certificate = certificate; > > Why not just use &m_certificate here? Ah! Nope. Only with GOwnPtr currently. Committed r143341: <http://trac.webkit.org/changeset/143341> |