Fix crash if multiple subresources on a page request http auth.

Bug: 1144209
Change-Id: Icef2ca844b3c0fb4e2704253890760d0fe5f36bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523357Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825128}

weblayer/BUILD.gn

@@ -193,8 +193,6 @@ source_set("weblayer_lib_base") {
     "browser/file_select_helper.h",
     "browser/host_content_settings_map_factory.cc",
     "browser/host_content_settings_map_factory.h",
-    "browser/http_auth_handler_impl.cc",
-    "browser/http_auth_handler_impl.h",
     "browser/i18n_util.cc",
     "browser/i18n_util.h",
     "browser/insecure_form_controller_client.cc",
@@ -538,6 +536,8 @@ source_set("weblayer_lib_base") {
       "browser/fullscreen_callback_proxy.h",
       "browser/google_accounts_callback_proxy.cc",
       "browser/google_accounts_callback_proxy.h",
+      "browser/http_auth_handler_impl.cc",
+      "browser/http_auth_handler_impl.h",
       "browser/infobar_container_android.cc",
       "browser/infobar_container_android.h",
       "browser/infobar_service.cc",

weblayer/browser/content_browser_client_impl.cc

@@ -78,7 +78,6 @@
 #include "weblayer/browser/download_manager_delegate_impl.h"
 #include "weblayer/browser/feature_list_creator.h"
 #include "weblayer/browser/host_content_settings_map_factory.h"
-#include "weblayer/browser/http_auth_handler_impl.h"
 #include "weblayer/browser/i18n_util.h"
 #include "weblayer/browser/navigation_controller_impl.h"
 #include "weblayer/browser/navigation_error_navigation_throttle.h"
@@ -127,6 +126,7 @@
 #include "weblayer/browser/android_descriptors.h"
 #include "weblayer/browser/browser_context_impl.h"
 #include "weblayer/browser/devtools_manager_delegate_android.h"
+#include "weblayer/browser/http_auth_handler_impl.h"
 #include "weblayer/browser/media/media_router_factory.h"
 #include "weblayer/browser/proxying_url_loader_factory_impl.h"
 #include "weblayer/browser/safe_browsing/real_time_url_lookup_service_factory.h"

weblayer/browser/http_auth_handler_impl.cc

@@ -4,9 +4,12 @@
 
 #include "weblayer/browser/http_auth_handler_impl.h"
 
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/auth.h"
+#include "weblayer/browser/java/jni/HttpAuthHandlerImpl_jni.h"
 #include "weblayer/browser/tab_impl.h"
 
 namespace weblayer {
@@ -22,29 +25,51 @@ HttpAuthHandlerImpl::HttpAuthHandlerImpl(
   url_ = auth_info.challenger.GetURL().Resolve(auth_info.path);
 
   auto* tab = TabImpl::FromWebContents(web_contents);
-  tab->ShowHttpAuthPrompt(this);
+  JNIEnv* env = base::android::AttachCurrentThread();
+  java_impl_ = Java_HttpAuthHandlerImpl_create(
+      env, reinterpret_cast<intptr_t>(this), tab->GetJavaTab(),
+      base::android::ConvertUTF8ToJavaString(env, url_.host()),
+      base::android::ConvertUTF8ToJavaString(env, url_.spec()));
 }
 
 HttpAuthHandlerImpl::~HttpAuthHandlerImpl() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  auto* tab = TabImpl::FromWebContents(web_contents());
-  if (tab)
-    tab->CloseHttpAuthPrompt();
+  CloseDialog();
+  if (java_impl_) {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_HttpAuthHandlerImpl_handlerDestroyed(env, java_impl_);
+    java_impl_ = nullptr;
+  }
+}
+
+void HttpAuthHandlerImpl::CloseDialog() {
+  if (!java_impl_)
+    return;
+
+  JNIEnv* env = base::android::AttachCurrentThread();
+  Java_HttpAuthHandlerImpl_closeDialog(env, java_impl_);
 }
 
-void HttpAuthHandlerImpl::Proceed(const base::string16& user,
-                                  const base::string16& password) {
+void HttpAuthHandlerImpl::Proceed(
+    JNIEnv* env,
+    const base::android::JavaParamRef<jstring>& username,
+    const base::android::JavaParamRef<jstring>& password) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (callback_) {
-    std::move(callback_).Run(net::AuthCredentials(user, password));
+    std::move(callback_).Run(net::AuthCredentials(
+        base::android::ConvertJavaStringToUTF16(env, username),
+        base::android::ConvertJavaStringToUTF16(env, password)));
   }
+
+  CloseDialog();
 }
 
-void HttpAuthHandlerImpl::Cancel() {
+void HttpAuthHandlerImpl::Cancel(JNIEnv* env) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  if (callback_) {
+  if (callback_)
     std::move(callback_).Run(base::nullopt);
-  }
+
+  CloseDialog();
 }
 
-}  // namespace weblayer
+}  // namespace weblayer
\ No newline at end of file

weblayer/browser/http_auth_handler_impl.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include <string>
 
+#include "base/android/scoped_java_ref.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/login_delegate.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -25,14 +26,17 @@ class HttpAuthHandlerImpl : public content::LoginDelegate,
                       LoginAuthRequiredCallback callback);
   ~HttpAuthHandlerImpl() override;
 
-  void Proceed(const base::string16& user, const base::string16& password);
-  void Cancel();
-
-  GURL url() { return url_; }
+  void Proceed(JNIEnv* env,
+               const base::android::JavaParamRef<jstring>& username,
+               const base::android::JavaParamRef<jstring>& password);
+  void Cancel(JNIEnv* env);
 
  private:
+  void CloseDialog();
+
   GURL url_;
   LoginAuthRequiredCallback callback_;
+  base::android::ScopedJavaGlobalRef<jobject> java_impl_;
 };
 
 }  // namespace weblayer

weblayer/browser/java/BUILD.gn

@@ -122,6 +122,7 @@ android_library("java") {
     "org/chromium/weblayer_private/FullscreenCallbackProxy.java",
     "org/chromium/weblayer_private/FullscreenToast.java",
     "org/chromium/weblayer_private/GoogleAccountsCallbackProxy.java",
+    "org/chromium/weblayer_private/HttpAuthHandlerImpl.java",
     "org/chromium/weblayer_private/InfoBarContainer.java",
     "org/chromium/weblayer_private/InfoBarContainerView.java",
     "org/chromium/weblayer_private/IntentUtils.java",
@@ -339,6 +340,7 @@ generate_jni("jni") {
     "org/chromium/weblayer_private/FaviconCallbackProxy.java",
     "org/chromium/weblayer_private/FullscreenCallbackProxy.java",
     "org/chromium/weblayer_private/GoogleAccountsCallbackProxy.java",
+    "org/chromium/weblayer_private/HttpAuthHandlerImpl.java",
     "org/chromium/weblayer_private/InfoBarContainer.java",
     "org/chromium/weblayer_private/LocaleChangedBroadcastReceiver.java",
     "org/chromium/weblayer_private/MojoInterfaceRegistrar.java",

weblayer/browser/java/org/chromium/weblayer_private/HttpAuthHandlerImpl.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer_private;
+
+import android.content.Context;
+
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
+import org.chromium.base.annotations.NativeMethods;
+import org.chromium.components.browser_ui.http_auth.LoginPrompt;
+
+/**
+ * Handles showing http auth prompt.
+ */
+@JNINamespace("weblayer")
+public final class HttpAuthHandlerImpl implements LoginPrompt.Observer {
+    private long mNativeHttpAuthHandlerImpl;
+    private LoginPrompt mLoginPrompt;
+
+    @CalledByNative
+    public static HttpAuthHandlerImpl create(
+            long nativeAuthHandler, TabImpl tab, String host, String url) {
+        return new HttpAuthHandlerImpl(nativeAuthHandler, tab.getBrowser().getContext(), host, url);
+    }
+
+    @CalledByNative
+    void handlerDestroyed() {
+        mNativeHttpAuthHandlerImpl = 0;
+    }
+
+    @CalledByNative
+    private void closeDialog() {
+        if (mLoginPrompt != null) mLoginPrompt.dismiss();
+    }
+
+    private HttpAuthHandlerImpl(
+            long nativeHttpAuthHandlerImpl, Context context, String host, String url) {
+        mNativeHttpAuthHandlerImpl = nativeHttpAuthHandlerImpl;
+
+        mLoginPrompt = new LoginPrompt(context, host, url, this);
+        mLoginPrompt.show();
+    }
+
+    @Override
+    public void proceed(String username, String password) {
+        if (mNativeHttpAuthHandlerImpl != 0) {
+            HttpAuthHandlerImplJni.get().proceed(mNativeHttpAuthHandlerImpl, username, password);
+        }
+    }
+
+    @Override
+    public void cancel() {
+        if (mNativeHttpAuthHandlerImpl != 0) {
+            HttpAuthHandlerImplJni.get().cancel(mNativeHttpAuthHandlerImpl);
+        }
+    }
+
+    @NativeMethods
+    interface Natives {
+        void proceed(long nativeHttpAuthHandlerImpl, String username, String password);
+        void cancel(long nativeHttpAuthHandlerImpl);
+    }
+}

weblayer/browser/java/org/chromium/weblayer_private/TabImpl.java

@@ -30,7 +30,6 @@ import org.chromium.base.annotations.NativeMethods;
 import org.chromium.components.autofill.AutofillActionModeCallback;
 import org.chromium.components.autofill.AutofillProvider;
 import org.chromium.components.browser_ui.display_cutout.DisplayCutoutController;
-import org.chromium.components.browser_ui.http_auth.LoginPrompt;
 import org.chromium.components.browser_ui.media.MediaSessionHelper;
 import org.chromium.components.browser_ui.util.BrowserControlsVisibilityDelegate;
 import org.chromium.components.browser_ui.util.ComposedBrowserControlsVisibilityDelegate;
@@ -87,7 +86,7 @@ import java.util.Set;
  * Implementation of ITab.
  */
 @JNINamespace("weblayer")
-public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
+public final class TabImpl extends ITab.Stub {
     private static int sNextId = 1;
     // Map from id to TabImpl.
     private static final Map<Integer, TabImpl> sTabMap = new HashMap<Integer, TabImpl>();
@@ -105,7 +104,6 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
     // BrowserImpl this TabImpl is in.
     @NonNull
     private BrowserImpl mBrowser;
-    private LoginPrompt mLoginPrompt;
     /**
      * The AutofillProvider that integrates with system-level autofill. This is null until
      * updateFromBrowser() is invoked.
@@ -870,27 +868,6 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
         getBrowser().destroyTab(this);
     }
 
-    @CalledByNative
-    private void showHttpAuthPrompt(String host, String url) {
-        mLoginPrompt = new LoginPrompt(mBrowser.getContext(), host, url, this);
-        mLoginPrompt.show();
-    }
-
-    @CalledByNative
-    private void closeHttpAuthPrompt() {
-        mLoginPrompt = null;
-    }
-
-    @Override
-    public void cancel() {
-        TabImplJni.get().cancelHttpAuth(mNativeTab);
-    }
-
-    @Override
-    public void proceed(String username, String password) {
-        TabImplJni.get().setHttpAuth(mNativeTab, username, password);
-    }
-
     @Override
     public void registerWebMessageCallback(
             String jsObjectName, List<String> allowedOrigins, IWebMessageCallbackClient client) {
@@ -1184,8 +1161,6 @@ public final class TabImpl extends ITab.Stub implements LoginPrompt.Observer {
         boolean setData(long nativeTabImpl, String[] data);
         String[] getData(long nativeTabImpl);
         boolean isRendererControllingBrowserControlsOffsets(long nativeTabImpl);
-        void setHttpAuth(long nativeTabImpl, String username, String password);
-        void cancelHttpAuth(long nativeTabImpl);
         String registerWebMessageCallback(long nativeTabImpl, String jsObjectName,
                 String[] allowedOrigins, IWebMessageCallbackClient client);
         void unregisterWebMessageCallback(long nativeTabImpl, String jsObjectName);

weblayer/browser/tab_impl.cc

@@ -101,7 +101,6 @@
 #include "weblayer/browser/browser_controls_container_view.h"
 #include "weblayer/browser/browser_controls_navigation_state_handler.h"
 #include "weblayer/browser/controls_visibility_reason.h"
-#include "weblayer/browser/http_auth_handler_impl.h"
 #include "weblayer/browser/java/jni/TabImpl_jni.h"
 #include "weblayer/browser/javascript_tab_modal_dialog_manager_delegate_android.h"
 #include "weblayer/browser/js_communication/web_message_host_factory_proxy.h"
@@ -548,28 +547,6 @@ void TabImpl::ShowContextMenu(const content::ContextMenuParams& params) {
 #endif
 }
 
-void TabImpl::ShowHttpAuthPrompt(HttpAuthHandlerImpl* auth_handler) {
-  CHECK(!auth_handler_);
-  auth_handler_ = auth_handler;
-#if defined(OS_ANDROID)
-  JNIEnv* env = AttachCurrentThread();
-  GURL url = auth_handler_->url();
-  Java_TabImpl_showHttpAuthPrompt(
-      env, java_impl_, base::android::ConvertUTF8ToJavaString(env, url.host()),
-      base::android::ConvertUTF8ToJavaString(env, url.spec()));
-#endif
-}
-
-void TabImpl::CloseHttpAuthPrompt() {
-  if (!auth_handler_)
-    return;
-  auth_handler_ = nullptr;
-#if defined(OS_ANDROID)
-  JNIEnv* env = AttachCurrentThread();
-  Java_TabImpl_closeHttpAuthPrompt(env, java_impl_);
-#endif
-}
-
 #if defined(OS_ANDROID)
 // static
 void TabImpl::DisableAutofillSystemIntegrationForTesting() {
@@ -771,21 +748,6 @@ jboolean TabImpl::IsRendererControllingBrowserControlsOffsets(JNIEnv* env) {
       ->IsRendererControllingOffsets();
 }
 
-void TabImpl::SetHttpAuth(
-    JNIEnv* env,
-    const base::android::JavaParamRef<jstring>& username,
-    const base::android::JavaParamRef<jstring>& password) {
-  auth_handler_->Proceed(
-      base::android::ConvertJavaStringToUTF16(env, username),
-      base::android::ConvertJavaStringToUTF16(env, password));
-  CloseHttpAuthPrompt();
-}
-
-void TabImpl::CancelHttpAuth(JNIEnv* env) {
-  auth_handler_->Cancel();
-  CloseHttpAuthPrompt();
-}
-
 base::android::ScopedJavaLocalRef<jstring> TabImpl::RegisterWebMessageCallback(
     JNIEnv* env,
     const base::android::JavaParamRef<jstring>& js_object_name,

weblayer/browser/tab_impl.h

@@ -62,7 +62,6 @@ class FullscreenDelegate;
 class NavigationControllerImpl;
 class NewTabDelegate;
 class ProfileImpl;
-class HttpAuthHandlerImpl;
 
 #if defined(OS_ANDROID)
 class BrowserControlsContainerView;
@@ -135,9 +134,6 @@ class TabImpl : public Tab,
 
   void ShowContextMenu(const content::ContextMenuParams& params);
 
-  void ShowHttpAuthPrompt(HttpAuthHandlerImpl* auth_handler);
-  void CloseHttpAuthPrompt();
-
 #if defined(OS_ANDROID)
   base::android::ScopedJavaGlobalRef<jobject> GetJavaTab() {
     return java_impl_;
@@ -184,10 +180,6 @@ class TabImpl : public Tab,
                    const base::android::JavaParamRef<jobjectArray>& data);
   base::android::ScopedJavaLocalRef<jobjectArray> GetData(JNIEnv* env);
   jboolean IsRendererControllingBrowserControlsOffsets(JNIEnv* env);
-  void SetHttpAuth(JNIEnv* env,
-                   const base::android::JavaParamRef<jstring>& username,
-                   const base::android::JavaParamRef<jstring>& password);
-  void CancelHttpAuth(JNIEnv* env);
   base::android::ScopedJavaLocalRef<jstring> RegisterWebMessageCallback(
       JNIEnv* env,
       const base::android::JavaParamRef<jstring>& js_object_name,
@@ -410,8 +402,6 @@ class TabImpl : public Tab,
 
   base::string16 title_;
 
-  HttpAuthHandlerImpl* auth_handler_ = nullptr;
-
   std::unique_ptr<js_injection::JsCommunicationHost> js_communication_host_;
 
   base::WeakPtrFactory<TabImpl> weak_ptr_factory_{this};