Commit c5c66361 authored by dominickn's avatar dominickn Committed by Commit bot

Don't show a modal permission prompt if the requesting WebContents has no tab.

Contextual search on Android spins up a WebContents which talks to
Google search. If Google search chooses to request a permission inside
this WebContents, the modal prompt crashes due to a lack of a containing
tab. This is not an issue with the standard infobar permission prompts
as they do not have a dependency on a tab for display.

This CL changes a DCHECK for a TabAndroid to an explicit conditional,
which acts as though the permission request was dismissed if there is no
active Tab for the requesting WebContents.

BUG=668640

Review-Url: https://codereview.chromium.org/2538253002
Cr-Commit-Position: refs/heads/master@{#435556}
parent e6dd62f7
......@@ -47,11 +47,17 @@ void PermissionDialogDelegate::Create(
const PermissionSetCallback& callback) {
DCHECK(web_contents);
// If we don't have a tab, just act as though the prompt was dismissed.
TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
if (!tab) {
callback.Run(false, PermissionAction::DISMISSED);
return;
}
// Dispatch the dialog to Java, which manages the lifetime of this object.
new PermissionDialogDelegate(
web_contents,
PermissionInfoBarDelegate::CreateDelegate(
type, requesting_frame, user_gesture, profile, callback));
tab, PermissionInfoBarDelegate::CreateDelegate(
type, requesting_frame, user_gesture, profile, callback));
}
// static
......@@ -59,6 +65,15 @@ void PermissionDialogDelegate::CreateMediaStreamDialog(
content::WebContents* web_contents,
bool user_gesture,
std::unique_ptr<MediaStreamDevicesController> controller) {
DCHECK(web_contents);
// If we don't have a tab, just act as though the prompt was dismissed.
TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
if (!tab) {
controller->Cancelled();
return;
}
// Called this way because the infobar delegate has a private destructor.
std::unique_ptr<PermissionInfoBarDelegate> infobar_delegate;
infobar_delegate.reset(new MediaStreamInfoBarDelegateAndroid(
......@@ -67,7 +82,7 @@ void PermissionDialogDelegate::CreateMediaStreamDialog(
std::move(controller)));
// Dispatch the dialog to Java, which manages the lifetime of this object.
new PermissionDialogDelegate(web_contents, std::move(infobar_delegate));
new PermissionDialogDelegate(tab, std::move(infobar_delegate));
}
// static
......@@ -91,15 +106,12 @@ bool PermissionDialogDelegate::RegisterPermissionDialogDelegate(JNIEnv* env) {
ScopedJavaLocalRef<jobject> PermissionDialogDelegate::CreateJavaDelegate(
JNIEnv* env) {
TabAndroid* tab = TabAndroid::FromWebContents(web_contents());
DCHECK(tab);
std::vector<int> content_settings_types{
infobar_delegate_->content_settings_types()};
return Java_PermissionDialogDelegate_create(
env, reinterpret_cast<uintptr_t>(this),
tab->GetJavaObject(),
tab_->GetJavaObject(),
base::android::ToJavaIntArray(env, content_settings_types).obj(),
ResourceMapper::MapFromChromiumId(infobar_delegate_->GetIconId()),
ConvertUTF16ToJavaString(env, infobar_delegate_->GetMessageText()),
......@@ -138,8 +150,8 @@ void PermissionDialogDelegate::LinkClicked(JNIEnv* env,
// Don't call delegate_->LinkClicked() because that relies on having an
// InfoBarService as an owner() to open the link. That will fail since the
// wrapped delegate has no owner (it hasn't been added as an infobar).
if (web_contents()) {
web_contents()->OpenURL(content::OpenURLParams(
if (tab_->web_contents()) {
tab_->web_contents()->OpenURL(content::OpenURLParams(
infobar_delegate_->GetLinkURL(), content::Referrer(),
WindowOpenDisposition::NEW_FOREGROUND_TAB, ui::PAGE_TRANSITION_LINK,
false));
......@@ -152,10 +164,10 @@ void PermissionDialogDelegate::Destroy(JNIEnv* env,
}
PermissionDialogDelegate::PermissionDialogDelegate(
content::WebContents* web_contents,
TabAndroid* tab,
std::unique_ptr<PermissionInfoBarDelegate> infobar_delegate)
: content::WebContentsObserver(web_contents),
infobar_delegate_(std::move(infobar_delegate)) {
: tab_(tab), infobar_delegate_(std::move(infobar_delegate)) {
DCHECK(tab_);
DCHECK(infobar_delegate_);
// Create our Java counterpart, which manages our lifetime.
......
......@@ -12,7 +12,6 @@
#include "base/macros.h"
#include "chrome/browser/permissions/permission_util.h"
#include "content/public/browser/permission_type.h"
#include "content/public/browser/web_contents_observer.h"
using base::android::JavaParamRef;
using base::android::ScopedJavaLocalRef;
......@@ -25,6 +24,7 @@ class MediaStreamDevicesController;
class GURL;
class PermissionInfoBarDelegate;
class Profile;
class TabAndroid;
// Delegate class for displaying a permission prompt as a modal dialog. Used as
// the native to Java interface to allow Java to communicate the user's
......@@ -35,7 +35,7 @@ class Profile;
// GroupedPermissionInfoBarDelegate, which will then source all of its data from
// an underlying PermissionPromptAndroid object. At that time, this class will
// also change to wrap a PermissionPromptAndroid.
class PermissionDialogDelegate : content::WebContentsObserver {
class PermissionDialogDelegate {
public:
using PermissionSetCallback = base::Callback<void(bool, PermissionAction)>;
......@@ -72,12 +72,14 @@ class PermissionDialogDelegate : content::WebContentsObserver {
private:
PermissionDialogDelegate(
content::WebContents* web_contents,
TabAndroid* tab,
std::unique_ptr<PermissionInfoBarDelegate> infobar_delegate_);
~PermissionDialogDelegate() override;
~PermissionDialogDelegate();
ScopedJavaLocalRef<jobject> CreateJavaDelegate(JNIEnv* env);
TabAndroid* tab_;
// The InfoBarDelegate which this class is wrapping.
// TODO(dominickn,lshang) replace this with PermissionPromptAndroid as the
// permission prompt refactoring continues.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment