Closed GoogleCodeExporter closed 9 years ago
Thanks for the patch. Some comments/questions:
1. Certificate errors for resources other than the main frame are suppressed
currently by the following code in
CefContentBrowserClient::AllowCertificateError:
if (resource_type != content::ResourceType::RESOURCE_TYPE_MAIN_FRAME) {
// A sub-resource has a certificate error. The user doesn't really
// have a context for making the right decision, so block the request
// hard.
*result = content::CERTIFICATE_REQUEST_RESULT_TYPE_CANCEL;
return;
}
This matches the behavior in Chrome (see
ChromeContentBrowserClient::AllowCertificateError). What is your use case for
allowing certificate errors from sub-resources? Which specific types of
sub-resources do you want to allow certificate errors for?
If certificate errors are allowed for sub-resources then the resource type
should also be passed to OnCertificateError so the application has all
necessary information to make an informed decision.
2. Line 22:
+ CefRefPtr<CefFrame> frame =
+ browser->GetFrame(render_frame_id);
No need to wrap lines that are less than 80 characters.
+ if (!frame.get())
+ return;
If the frame ID is unknown (GetFrame returns NULL) we should pass the main
frame (browser->GetMainFrame()) instead of not executing the callback.
3. Pass the frame argument to LoadErrorPage in
ClientHandler::OnCertificateError.
4. Create patch files using the `git diff --no-prefix` command (this removes
the a/ b/ prefixes from the file paths).
Original comment by magreenb...@gmail.com
on 6 Mar 2015 at 6:54
@#1: Note that if certificate errors are not allowed for sub-resources then
there is no need to pass the frame as an argument since the callback will only
ever be executed for the main frame.
Original comment by magreenb...@gmail.com
on 6 Mar 2015 at 7:00
Marshall, we reviewed again our use-case, and now that we know that this
callback occurs only on the main frame - it really seems a bit silly to pass
the frame.
The idea was to provide an indication to the user that a frame had an error
(even it it was an iframe), but since chrome doesn't do this - it's probably OK
for us to ignore such errors.
Thanks for you time.
Original comment by amitkan...@gmail.com
on 8 Mar 2015 at 5:09
Original comment by magreenb...@gmail.com
on 9 Mar 2015 at 5:01
Original issue reported on code.google.com by
amitkan...@gmail.com
on 3 Mar 2015 at 10:10Attachments: