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

chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm

Index: chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm
diff --git a/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm b/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm
index 61bd246166717a1e644ef40c9ad7ad491b978620..fb8f01ffd90a99a27b3bae86fd3cb3ea83a9a3db 100644
--- a/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm
+++ b/chrome/browser/ui/cocoa/website_settings/permission_bubble_cocoa.mm
@@ -12,9 +12,7 @@
 #import "ui/base/cocoa/nsview_additions.h"
 
 PermissionBubbleCocoa::PermissionBubbleCocoa(Browser* browser)
-    : browser_(browser), delegate_(nullptr), bubbleController_(nil) {
-  DCHECK(browser);
-}
+    : browser_(browser), delegate_(nullptr), bubbleController_(nil) {}

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

Why is the DCHECK moved?

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

A number of browser and unit tests do navigations, which run through
PermissionRequestManager::DidNavigateMainFrame, which calls FinalizeBubble,
which now creates a bubble if one doesn't exist. In many cases, a null browser
pointer is sent into the constructor here (and in the views impl too).

To fix those tests, I moved the DCHECK to the methods which actually need the
non-null pointer. This means that the tests work, and we still ensure that any
test which tries to show or otherwise do UI-related things to bubbles still have
the DCHECK.

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

Sounds good.

 
 PermissionBubbleCocoa::~PermissionBubbleCocoa() {
 }
@@ -22,6 +20,8 @@ PermissionBubbleCocoa::~PermissionBubbleCocoa() {
 void PermissionBubbleCocoa::Show(
     const std::vector<PermissionRequest*>& requests,
     const std::vector<bool>& accept_state) {
+  DCHECK(browser_);
+
   if (!bubbleController_) {
     bubbleController_ =
         [[PermissionBubbleController alloc] initWithBrowser:browser_