[Mac] Address buggy permission bubble behaviour on dismissal via ESC.

chrome/browser/permissions/permission_request_manager.cc

Index: chrome/browser/permissions/permission_request_manager.cc
diff --git a/chrome/browser/permissions/permission_request_manager.cc b/chrome/browser/permissions/permission_request_manager.cc
index 19d67cfc7acbf04645a87a951b6d16143be8b04e..ff56b127627c7b79f158a3494d9ec8d754660010 100644
--- a/chrome/browser/permissions/permission_request_manager.cc
+++ b/chrome/browser/permissions/permission_request_manager.cc
@@ -223,6 +223,12 @@ void PermissionRequestManager::CancelRequest(PermissionRequest* request) {
   NOTREACHED();  // Callers should not cancel requests that are not pending.
 }
 
+void PermissionRequestManager::DismissBubble() {

[hcarmona, 2016/10/10 18:08:19]

|Closing| calls |FinalizeBubble| which hides the bubble. Can we use that
instead of creating a new method?

[dominickn, 2016/10/10 22:55:48]

This would work, and I did think of doing that. The main caveat is that
|Closing| is private and an inherited delegate method within
PermissionRequestManager, so BrowserWindowController can't currently access it.
I thought about exposing it - in that case, it would be the only public delegate
method exposed in this way. felt, do you have OWNER thoughts on this?

[hcarmona, 2016/10/11 14:45:33]

Casting to PermissionPrompt::Delegate will let you call |Closing|, this
way you don't need to expose the method.

Maybe having |DismissBubble| just call |Closing| so it's more obvious
that this is supposed to do the same thing?

However, I defer to felt@ on what is best to do here.

+  if (IsBubbleVisible())
+    Closing();
+  HideBubble();
+}
+
 void PermissionRequestManager::HideBubble() {
   // Disengage from the existing view if there is one.
   if (!view_)
@@ -395,8 +401,18 @@ void PermissionRequestManager::TriggerShowBubble() {
 }
 
 void PermissionRequestManager::FinalizeBubble() {
-  if (view_)
+  if (view_) {
     view_->Hide();
+#if !defined(OS_ANDROID)
+  } else if (web_contents() && !web_contents()->IsBeingDestroyed()) {
+    // Ensure that the view exists. It may have been deleted by HideBubble,

[hcarmona, 2016/10/10 18:08:19]

Good description. However, since your change is fixing this, can we
remove the crbug reference? Whenever I see a link to the bug tracker I
assume it's an active bug.

[dominickn, 2016/10/10 22:55:47]

Done.

+    // creating a race condition where if AddRequest is called before
+    // DisplayPendingRequests, no prompt will be displayed. See
+    // crbug.com/653498.
+    view_ = view_factory_.Run(web_contents());
+    view_->SetDelegate(this);
+#endif
+  }
 
   std::vector<PermissionRequest*>::iterator requests_iter;
   for (requests_iter = requests_.begin();