New issue
Advanced search Search tips

Issue 736867 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

No integration tests making sure net::CertVerifyProcAndroid::SetCertNetFetcher is called.

Project Member Reported by mmenke@chromium.org, Jun 26 2017

Issue description

I accidentally removed the call to net::CertVerifyProcAndroid::SetCertNetFetcher in https://codereview.chromium.org/2929153002.  The change was missed because I stopped using the SYstemURLRequestContext class, but forgot to delete it, and no tests failed.

Surely if we depend on this being hooked up, we should have a browser test that covers this?  This is very likely to break against as part of network servification, unless we add tests for it in the meantime.
 

Comment 1 by est...@chromium.org, Jun 27 2017

Cc: -est...@chromium.org
Components: Test>Missing
Owner: est...@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by est...@chromium.org, Jul 31 2017

Braindump: I looked into what we need to do here and there are a couple steps. AFAICT InProcessBrowserTests don't run on Android, so I think we need to write this test in Java. The steps are:

1.) SpawnedTestServer supports a CERT_AUTO_AIA_INTERMEDIATE option that will be helpful in an integration test for CertVerifyProcAndroid::SetCertNetFetcher. However, that option isn't available in EmbeddedTestServer due to a lack of demand. We need to make it available in EmbeddedTestServer so that we can expose it to Java.
2.) Expose EmbeddedTestServer HTTPS options to Java. I'm not 100% sure this will work out of the box because EmbeddedTestServer runs in a separate process on Android, so I'm not sure if all the certificate setup will work, needs more investigation...
3.) Write a Java test that navigates to a CERT_AUTO_AIA_INTERMEDIATE EmbeddedTestServer URL and checks that there is no certificate error.

Comment 3 by est...@chromium.org, Jul 31 2017

(I'm planning to get a start on this later this week, just wanted to record my notes about what needs to be done)

Comment 4 by mmenke@chromium.org, Jul 31 2017

Looks like we do run content_browsertests on Android, though we currently don't call SetCertNetFetcher in content_shell.  But we'll have to hook it up to the network service, which both content/ and chrome/ will use, so may be an option to test it in content_shell instead (Erm...after hooking it up...What fun)
Status: Started (was: Assigned)
We need #2 in comment 2 (support EmbeddedTestServer's HTTPS options in Java tests) for various things, so I've gone ahead and started on that path.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92

commit 8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92
Author: Emily Stark <estark@google.com>
Date: Wed Aug 09 22:58:22 2017

Expose EmbeddedTestServer's HTTPS abilities to Java

This change allows Java users of EmbeddedTestServer to configure the server to
run HTTPS, using the same ServerCertificate options that are available to C++
users. This will allow us to write integration tests for security UI and will
allow the replacement of some TestWebServer uses.

One complication is that, for Android tests, EmbeddedTestServer runs in a
separate process, so its method of installing test root certs doesn't
work out of the box. To make this work, I've made the test process
server ask the out-of-process server for its root certificate, which it
then installs in a test root store.

As a first user of this functionality, I've re-enabled an old flaky omnibox
test. The test originally was hitting a real external url (google.com) to test
HTTPS, which I assume was the source of the flakiness. I've rewritten it to hit
an EmbeddedTestServer HTTPS URL, and updated it for recent UI changes.

Bug:  750343 , 736867
Change-Id: I2f98924d6b9998bc75ec2f74bd8ad5582942cb28
Reviewed-on: https://chromium-review.googlesource.com/598510
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493168}
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/chrome/android/javatests/src/org/chromium/chrome/browser/WarmupManagerTest.java
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/OmniboxTest.java
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/android/BUILD.gn
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServer.java
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/android/javatests/src/org/chromium/net/test/EmbeddedTestServerImpl.java
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/android/javatests/src/org/chromium/net/test/IEmbeddedTestServerImpl.aidl
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/embedded_test_server/android/embedded_test_server_android.cc
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/embedded_test_server/android/embedded_test_server_android.h
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/embedded_test_server/embedded_test_server.cc
[modify] https://crrev.com/8f0f8f4ae8d1a4f357dc39e4d3861e54dffe4d92/net/test/embedded_test_server/embedded_test_server.h

Components: Tests>Missing
Components: -Test>Missing

Sign in to add a comment