[WebsiteSettings] Fix bug in permission bubble manager to alter showing state when bubble is closed.

[WebsiteSettings] Fix bug in permission bubble manager to alter showing state when bubble is closed.

This bug meant that subsequent calls to AddRequest wouldn't show the bubble, since the manager still believed it was being shown.

R=leng@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251397

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252292

[leng, 2014-02-13 01:12:45]

lgtm

Could you add a description of the bug this fixes?  I suspect it was never
showing a second bubble, but it would be nice to have it called out.

https://codereview.chromium.org/162423002/diff/1/chrome/browser/ui/website_se...
File chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc
(right):

https://codereview.chromium.org/162423002/diff/1/chrome/browser/ui/website_se...
chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc:196:
manager_->AddRequest(&request2_);
It's not clear to me why you're adding request2 twice, because you're not
checking that it wasn't shown, or granted.  Perhaps add
EXPECT_FALSE(request2_.granted_); after EXPECT_TRUE(request1_.granted_)?

[Greg Billock, 2014-02-13 01:39:22]

+markusheintz for owners

https://codereview.chromium.org/162423002/diff/1/chrome/browser/ui/website_se...
File chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc
(right):

https://codereview.chromium.org/162423002/diff/1/chrome/browser/ui/website_se...
chrome/browser/ui/website_settings/permission_bubble_manager_unittest.cc:196:
manager_->AddRequest(&request2_);
On 2014/02/13 01:12:45, leng wrote:
> It's not clear to me why you're adding request2 twice, because you're not
> checking that it wasn't shown, or granted.  Perhaps add
> EXPECT_FALSE(request2_.granted_); after EXPECT_TRUE(request1_.granted_)?

Thanks. This was just exercising a log line to make sure the bug was there and
that this fixed it.

[markusheintz_, 2014-02-13 18:04:22]

LGTM

[Greg Billock, 2014-02-13 18:23:40]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-02-13 18:25:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/50001

[commit-bot: I haz the power, 2014-02-13 18:25:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-13 18:25:55]

Failed to apply patch for
chrome/browser/ui/website_settings/permission_bubble_manager.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/website_settings/permission_bubble_manager.cc
  Hunk #1 FAILED at 32.
  Hunk #2 succeeded at 169 with fuzz 2 (offset 45 lines).
  1 out of 2 hunks FAILED -- saving rejects to file
chrome/browser/ui/website_settings/permission_bubble_manager.cc.rej

Patch:       chrome/browser/ui/website_settings/permission_bubble_manager.cc
Index: chrome/browser/ui/website_settings/permission_bubble_manager.cc
diff --git a/chrome/browser/ui/website_settings/permission_bubble_manager.cc
b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
index
cb3df39d6b39ba73206ca70d492a2a430354a009..0e6a3bebf1ef7bef30c901874ec3ce05b806397d
100644
--- a/chrome/browser/ui/website_settings/permission_bubble_manager.cc
+++ b/chrome/browser/ui/website_settings/permission_bubble_manager.cc
@@ -32,7 +32,6 @@ void
PermissionBubbleManager::AddRequest(PermissionBubbleRequest* request) {
   // with various states of the manager.
 
   if (view_ && !bubble_showing_) {
-    view_->SetDelegate(this);
     view_->Show(requests_, accept_state_, customization_mode_);
     bubble_showing_ = true;
   }
@@ -124,10 +123,9 @@ void PermissionBubbleManager::Closing() {
 }
 
 void PermissionBubbleManager::FinalizeBubble() {
-  if (view_) {
-    view_->SetDelegate(NULL);
+  if (view_)
     view_->Hide();
-  }
+  bubble_showing_ = false;
 
   std::vector<PermissionBubbleRequest*>::iterator di;
   for (di = requests_.begin(); di != requests_.end(); di++)

[Greg Billock, 2014-02-13 19:56:01]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-02-13 20:10:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/160001

[Greg Billock, 2014-02-13 21:06:28]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-02-13 21:38:13]

Failed to trigger a try job on linux_chromeos
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2014-02-13 21:39:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/380001

[commit-bot: I haz the power, 2014-02-13 22:42:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-13 22:42:38]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Greg Billock, 2014-02-13 22:52:21]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-02-13 22:53:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/380001

[commit-bot: I haz the power, 2014-02-14 02:53:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 02:53:35]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[Greg Billock, 2014-02-14 18:40:01]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-02-14 18:41:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/380001

[commit-bot: I haz the power, 2014-02-14 19:48:51]

Change committed as 251397

[Greg Billock, 2014-02-18 18:27:23]

On 2014/02/14 19:48:51, I haz the power (commit-bot) wrote:
> Change committed as 251397

[ RUN      ] ImmersiveModeControllerAshTest.TabAndBrowserFullscreen
Xlib:  extension "RANDR" missing on display ":9".
[21038:21038:0214/121606:1047752665:WARNING:native_view_host_aura.cc(71)]
NativeViewHostAura::InstallClip is not implemented yet.
[21038:21038:0214/121606:1047752835:WARNING:native_view_host_aura.cc(71)]
NativeViewHostAura::InstallClip is not implemented yet.
[21038:21038:0214/121606:1047752914:WARNING:native_view_host_aura.cc(71)]
NativeViewHostAura::InstallClip is not implemented yet.
Received signal 11 SEGV_MAPERR 00020000002d
 [0x7f0b3aa767ce] base::debug::StackTrace::StackTrace()
 [0x7f0b3aa76cf8] base::debug::(anonymous namespace)::StackDumpSignalHandler()
 [0x7f0b34340cb0] \u003Cunknown>
 [0x00000379579c] PermissionBubbleViewViews::~PermissionBubbleViewViews()

Looks like the same problem as in 162713003. Going to get that going first.

[Greg Billock, 2014-02-20 00:10:37]

The CQ bit was checked by gbillock@chromium.org

[commit-bot: I haz the power, 2014-02-20 00:13:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/850001

[commit-bot: I haz the power, 2014-02-20 03:06:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/850001

[commit-bot: I haz the power, 2014-02-20 05:22:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/850001

[commit-bot: I haz the power, 2014-02-20 09:23:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/850001

[commit-bot: I haz the power, 2014-02-20 12:37:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gbillock@chromium.org/162423002/850001

[commit-bot: I haz the power, 2014-02-20 17:33:40]

Change committed as 252292