b/2511635 Browser displays incorrect SSL cert information

Two more cases of "View certificate" problems like b/2511635

One problem is that if there are multiple resources downloaded for a
page. In that case the mCertificate shown ends up being from the last
loaded resource instead of the main resource of the page. The solution
is to only set the certificate if the LoadListener is the
mIsMainResourceLoader as well as the mIsMainPageLoader.

A larger problem was the fact that the EventHandler.certificate
interface method (in this case the LoadListener.certificate
implementation) once per https connection instead of once per request
as was documented. That meant if an https connection was reused (which
happens frequently on login pages such as
https://www.google.com/accounts which use the POST -> redirect -> GET
idiom to avoid POST data page refresh warnings) then later pages never
were associated with an SslCertificate.

The solution was to change EventHandler.certificate to be called once
per request, specifcally before the request. This means we no longer
call the certificate method in the handleSslErrorRequest case, which
is okay because it includes the SslCertificate within the SslError and
that is what the BrowserActivity expects.

Change-Id: Icbd9bd98c89db82762d1d06de85e1cde2470300d

6 files changed, 32 insertions, 20 deletions

diff --git a/core/java/android/net/http/Connection.java b/core/java/android/net/http/Connection.java
index b8e17da..43fb5f1 100644
--- a/core/java/android/net/http/Connection.java
+++ b/core/java/android/net/http/Connection.java
@@ -222,6 +222,12 @@ abstract class Connection {
                         }
                     }
 
+                    /* we have a connection, let the event handler
+                     * know of any associated certificate,
+                     * potentially none.
+                     */
+                    req.mEventHandler.certificate(mCertificate);
+
                     try {
                         /* FIXME: don't increment failure count if old
                            connection?  There should not be a penalty for
diff --git a/core/java/android/net/http/EventHandler.java b/core/java/android/net/http/EventHandler.java
index a035c19..2aa05eb 100644
--- a/core/java/android/net/http/EventHandler.java
+++ b/core/java/android/net/http/EventHandler.java
@@ -125,8 +125,8 @@ public interface EventHandler {
     public void endData();
 
     /**
-     * SSL certificate callback called every time a resource is
-     * loaded via a secure connection
+     * SSL certificate callback called before resource request is
+     * made, which will be null for insecure connection.
      */
     public void certificate(SslCertificate certificate);
 
diff --git a/core/java/android/net/http/HttpsConnection.java b/core/java/android/net/http/HttpsConnection.java
index a67fd9a..e512a1df 100644
--- a/core/java/android/net/http/HttpsConnection.java
+++ b/core/java/android/net/http/HttpsConnection.java
@@ -308,12 +308,6 @@ public class HttpsConnection extends Connection {
         SslError error = CertificateChainValidator.getInstance().
             doHandshakeAndValidateServerCertificates(this, sslSock, mHost.getHostName());
 
-        EventHandler eventHandler = req.getEventHandler();
-
-        // Update the certificate info (to be consistent, it is better to do it
-        // here, before we start handling SSL errors, if any)
-        eventHandler.certificate(mCertificate);
-
         // Inform the user if there is a problem
         if (error != null) {
             // handleSslErrorRequest may immediately unsuspend if it wants to
@@ -325,7 +319,7 @@ public class HttpsConnection extends Connection {
                 mSuspended = true;
             }
             // don't hold the lock while calling out to the event handler
-            boolean canHandle = eventHandler.handleSslErrorRequest(error);
+            boolean canHandle = req.getEventHandler().handleSslErrorRequest(error);
             if(!canHandle) {
                 throw new IOException("failed to handle "+ error);
             }
diff --git a/core/java/android/webkit/BrowserFrame.java b/core/java/android/webkit/BrowserFrame.java
index af42ce0..dae9187 100644
--- a/core/java/android/webkit/BrowserFrame.java
+++ b/core/java/android/webkit/BrowserFrame.java
@@ -586,7 +586,10 @@ class BrowserFrame extends Handler {
      * @param headers The http headers.
      * @param postData If the method is "POST" postData is sent as the request
      *                 body. Is null when empty.
-     * @param cacheMode The cache mode to use when loading this resource.
+     * @param postDataIdentifier If the post data contained form this is the form identifier, otherwise it is 0.
+     * @param cacheMode The cache mode to use when loading this resource. See WebSettings.setCacheMode
+     * @param mainResource True if the this resource is the main request, not a supporting resource
+     * @param userGesture
      * @param synchronous True if the load is synchronous.
      * @return A newly created LoadListener object.
      */
diff --git a/core/java/android/webkit/LoadListener.java b/core/java/android/webkit/LoadListener.java
index a23a4ce..dc7723f 100644
--- a/core/java/android/webkit/LoadListener.java
+++ b/core/java/android/webkit/LoadListener.java
@@ -121,6 +121,7 @@ class LoadListener extends Handler implements EventHandler {
 
     // Does this loader correspond to the main-frame top-level page?
     private boolean mIsMainPageLoader;
+    // Does this loader correspond to the main content (as opposed to a supporting resource)
     private final boolean mIsMainResourceLoader;
     private final boolean mUserGesture;
 
@@ -520,23 +521,28 @@ class LoadListener extends Handler implements EventHandler {
     }
 
     /**
-     * Implementation of certificate handler for EventHandler.
-     * Called every time a resource is loaded via a secure
-     * connection. In this context, can be called multiple
-     * times if we have redirects
-     * @param certificate The SSL certifcate
-     * IMPORTANT: as this is called from network thread, can't call native
-     * directly
+     * Implementation of certificate handler for EventHandler. Called
+     * before a resource is requested. In this context, can be called
+     * multiple times if we have redirects
+     *
+     * IMPORTANT: as this is called from network thread, can't call
+     * native directly
+     *
+     * @param certificate The SSL certifcate or null if the request
+     * was not secure
      */
     public void certificate(SslCertificate certificate) {
+        if (DebugFlags.LOAD_LISTENER) {
+            Log.v(LOGTAG, "LoadListener.certificate: " + certificate);
+        }
         sendMessageInternal(obtainMessage(MSG_SSL_CERTIFICATE, certificate));
     }
 
     // Handle the certificate on the WebCore thread.
     private void handleCertificate(SslCertificate certificate) {
-        // if this is the top-most main-frame page loader
-        if (mIsMainPageLoader) {
-            // update the browser frame (ie, the main frame)
+        // if this is main resource of the top frame
+        if (mIsMainPageLoader && mIsMainResourceLoader) {
+            // update the browser frame with certificate
             mBrowserFrame.certificate(certificate);
         }
     }
diff --git a/core/java/android/webkit/WebView.java b/core/java/android/webkit/WebView.java
index 8f7bf8d..8ce6276 100644
--- a/core/java/android/webkit/WebView.java
+++ b/core/java/android/webkit/WebView.java
@@ -1088,6 +1088,9 @@ public class WebView extends AbsoluteLayout
      * Sets the SSL certificate for the main top-level page.
      */
     public void setCertificate(SslCertificate certificate) {
+        if (DebugFlags.WEB_VIEW) {
+            Log.v(LOGTAG, "setCertificate=" + certificate);
+        }
         // here, the certificate can be null (if the site is not secure)
         mCertificate = certificate;
     }