Create/Destroy PermissionPromptAndroid when switching tabs on Android

chrome/browser/ui/android/tab_model/tab_model_selector_base.cc

+// Copyright 2016 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.
+
+#include "chrome/browser/ui/android/tab_model/tab_model_selector_base.h"
+
+#include "base/android/jni_android.h"
+#include "chrome/browser/android/tab_android.h"
+#include "chrome/browser/permissions/permission_request_manager.h"
+#include "content/public/browser/web_contents.h"
+#include "jni/TabModelSelectorBase_jni.h"
+
+using base::android::JavaParamRef;
+
+static void OnActiveTabChanged(
+    JNIEnv* env,
+    const JavaParamRef<jclass>& clazz,
+    const JavaParamRef<jobject>& java_old_web_contents,
+    const JavaParamRef<jobject>& java_new_web_contents) {
+  // Check if PermissionRequestManager is reused on Android to manage infobars.
+  // This is currently behind a switch flag.
+  if (PermissionRequestManager::IsEnabled()) {

[Bernhard Bauer, 2016/11/24 15:30:40]

Return early if it's disabled?

[lshang, 2016/11/25 00:41:06]

Done.

+    content::WebContents* old_contents =
+        content::WebContents::FromJavaWebContents(java_old_web_contents);
+    content::WebContents* new_contents =
+        content::WebContents::FromJavaWebContents(java_new_web_contents);
+    DCHECK(new_contents);
+
+    if (old_contents && PermissionRequestManager::FromWebContents(old_contents))
+      PermissionRequestManager::FromWebContents(old_contents)->HideBubble();

[Bernhard Bauer, 2016/11/24 15:30:40]

When is the PermissionRequestManager null?

[lshang, 2016/11/25 00:41:06]

I guess
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/browser_vi...
checks it for some reason (or just avoid crashes in case).

I can also find some evidence in
https://cs.chromium.org/chromium/src/chrome/browser/permissions/permission_co...
that PermissionRequestManager can be null in some edge cases.

[Bernhard Bauer, 2016/11/25 09:52:24]

Please don't try to avoid crashes "just in case"; that just leads to
cargo-culting. If you think it genuinely can't be null, just leave out the check
-- if the assumption turns out to be wrong, you'll get crash reports. OTOH, if
you have the null-check in here in an attempt to be safe but it is superfluous,
you'll never learn about it.

In this particular case, PermissionRequestManager is WebContentsUserData, and it
is created in TabHelpers::AttachTabHelpers(). It is true that there might be
WebContents that don't have these tab helpers attached (anything that is not
shown in a tab, for example), but given that this code path is called from the
Java TabModel, it seems unlikely that that would apply here.

+
+    if (new_contents &&
+        PermissionRequestManager::FromWebContents(new_contents)) {
+      PermissionRequestManager::FromWebContents(new_contents)
+          ->DisplayPendingRequests();
+    }
+  }
+}
+
+// Register native methods
+bool RegisterTabModelSelectorBase(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}