Remove pointer from PermissionPromptAndroid to InfoBar

chrome/browser/permissions/permission_prompt_android.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/permissions/permission_prompt_android.h"
 
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/infobars/infobar_service.h"
-#include "chrome/browser/media/webrtc/media_stream_devices_controller.h"
 #include "chrome/browser/permissions/grouped_permission_infobar_delegate_android.h"
 #include "chrome/browser/permissions/permission_request.h"
-#include "chrome/browser/permissions/permission_request_id.h"
-#include "components/infobars/core/infobar.h"
-#include "content/public/browser/web_contents.h"
 
 PermissionPromptAndroid::PermissionPromptAndroid(
     content::WebContents* web_contents)
-    : web_contents_(web_contents), delegate_(nullptr), infobar_(nullptr) {
+    : web_contents_(web_contents), delegate_(nullptr) {
   DCHECK(web_contents);
 }
 
-PermissionPromptAndroid::~PermissionPromptAndroid() {
-  if (infobar_) {
-    GroupedPermissionInfoBarDelegate* infobar_delegate =
-        static_cast<GroupedPermissionInfoBarDelegate*>(infobar_->delegate());
-    infobar_delegate->PermissionPromptDestroyed();
-  }
-}
+PermissionPromptAndroid::~PermissionPromptAndroid() {}
 
 void PermissionPromptAndroid::SetDelegate(Delegate* delegate) {
   delegate_ = delegate;
 }
 
 void PermissionPromptAndroid::Show(
     const std::vector<PermissionRequest*>& requests,
     const std::vector<bool>& values) {
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(web_contents_);
   if (!infobar_service)
     return;
 
   requests_ = requests;
-  infobar_ = GroupedPermissionInfoBarDelegate::Create(this, infobar_service,
-                                                      requests[0]->GetOrigin());
+  GroupedPermissionInfoBarDelegate::Create(this, infobar_service,
+                                           requests[0]->GetOrigin());
 }
 
 bool PermissionPromptAndroid::CanAcceptRequestUpdate() {
   return false;
 }
 
 void PermissionPromptAndroid::Hide() {
-  InfoBarService* infobar_service =
-      InfoBarService::FromWebContents(web_contents_);
-  if (infobar_ && infobar_service) {
-    infobar_service->RemoveInfoBar(infobar_);
-  }
-  infobar_ = nullptr;
+  // No-op. Hide() is called in three cases:
+  // - After the user resolves a prompt. The infobar manager will hide the
+  // prompt for us.
+  // - DidFinishNavigation, etc. The infobar manager handles this.
+  // - PermissionManager::CancelRequest(). As CanAcceptRequestUpdate() returns
+  // false, it doesn't call into here and instead dummies out the callbacks.

[benwells, 2017/03/20 03:00:43]

This feels brittle - e.g. someone might add a new call to Hide and things won't
work.

Can you think of a better solution that doesn't rely on callers of Hide all
being unnecessary for infobars?

[Timothy Loh, 2017/03/20 04:00:41]

If we don't remove this pointer, then we'd need to add more logic to correctly
maintain the state of the pointer (of note is we'd need to add an
InfoBarManager::Observer so we can know when it's removed due to navigation).
And we'd probably want to add a flag to Hide() since it's weird for the
PermissionRequestManager to also try and remove infobars when the InfoBarManager
should really be in charge of it.

It seems to me that it's simpler to remove this pointer than to try and make it
actually hide the infobar for future potential usages (YAGNI).

 }
 
 bool PermissionPromptAndroid::IsVisible() {
-  return infobar_ != nullptr;
+  return !requests_.empty();
 }
 
 void PermissionPromptAndroid::UpdateAnchorPosition() {
   NOTREACHED() << "UpdateAnchorPosition is not implemented";
 }
 
 gfx::NativeWindow PermissionPromptAndroid::GetNativeWindow() {
   NOTREACHED() << "GetNativeWindow is not implemented";
   return nullptr;
 }
@@ skipping 36 matching lines @@
     size_t position) const {
   DCHECK_LT(position, requests_.size());
   return requests_[position]->GetMessageTextFragment();
 }
 
 // static
 std::unique_ptr<PermissionPrompt> PermissionPrompt::Create(
     content::WebContents* web_contents) {
   return base::MakeUnique<PermissionPromptAndroid>(web_contents);
 }