disable WebView SSL exception when command line flag ignore-certificate-errors is given

disable WebView SSL exception when command line flag ignore-certificate-errors is given

BUG=396157
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289497

[wuhu, 2014-07-29 00:12:33]

Can you please take a look the change when you have time?
Thanks

[mnaganov (inactive), 2014-07-29 08:24:48]

https://codereview.chromium.org/427673006/diff/1/android_webview/native/aw_co...
File android_webview/native/aw_contents_client_bridge.cc (right):

https://codereview.chromium.org/427673006/diff/1/android_webview/native/aw_co...
android_webview/native/aw_contents_client_bridge.cc:111: } else if
(CommandLine::ForCurrentProcess()->HasSwitch(
It looks like you'll end up with error callbacks piling up in the map because
you are not removing them. How about this flow:

if (!callback ...) {
  ...
}
if (!HasSwitch(...)) {
  callback->Run(proceed);
} else {
  LOG ...
}
pending_cert_error_callbacks_.Remove(id);

[benm (inactive), 2014-07-29 09:56:42]

+sugrun FYI

[tonyg, 2014-07-29 17:39:11]

lg2m after Mikhail's comment is addressed.

[wuhu, 2014-07-29 21:31:57]

On 2014/07/29 17:39:11, tonyg wrote:
> lg2m after Mikhail's comment is addressed.

Thanks Mikhail and Tony for the great feedback.  After looking it a bit more
closely, there seems to be a more logical place for putting this check
(aw_url_request_context_getter.cc).  This is is upstream of the initial patch
and groups the all command line parameter processing in one location.  It's also
consistent with how the same flag (ignore-certificate-error) is handled in
chrome/clank.  What do you guys think?

[sgurun-gerrit only, 2014-07-29 22:21:19]

sounds right location. the previous was a little bit awkward.

https://codereview.chromium.org/427673006/diff/20001/android_webview/browser/...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/427673006/diff/20001/android_webview/browser/...
android_webview/browser/net/aw_url_request_context_getter.cc:84:
params->ignore_certificate_errors = false;
default is false, so no need to do it (and it seems for params we rely on the
default value and only set to overriding value here).

[wuhu, 2014-08-07 18:43:57]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-08-07 18:46:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wuhu@google.com/427673006/20001

[commit-bot: I haz the power, 2014-08-07 18:46:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-07 18:46:36]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[sgurun-gerrit only, 2014-08-11 20:58:15]

On 2014/08/07 18:46:36, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet. Only full committers are accepted.
> Even if an LGTM may have been provided, it was from a non-committer or
> a provisional committer, _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

hello. you need somebody's approval for submitting. can you please address my
comments above? thx

[wuhu, 2014-08-12 00:15:17]

My apologies, the patch has been updated.  Thanks

[sgurun-gerrit only, 2014-08-13 21:46:17]

On 2014/08/12 00:15:17, wuhu wrote:
> My apologies, the patch has been updated.  Thanks

lgtm

[wuhu, 2014-08-13 23:23:35]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-08-13 23:26:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wuhu@google.com/427673006/40001

[commit-bot: I haz the power, 2014-08-14 09:17:26]

Committed patchset #3 (40001) as 289497