Closes zoom bubble when tab is changed within a browser window.

Closes zoom bubble when tab is changed within a browser window.

BUG=143209
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=151962

[Peter Kasting, 2012-08-15 19:19:20]

Note that this will get called in more cases than just when the tab is changed. 
We'll also call this on URL changes within the tab, even when the user is typing
in the omnibox and thus won't actually see the URL change.

As long as you're fine with that, LGTM.

https://chromiumcodereview.appspot.com/10854169/diff/1/chrome/browser/ui/view...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://chromiumcodereview.appspot.com/10854169/diff/1/chrome/browser/ui/view...
chrome/browser/ui/views/location_bar/location_bar_view.cc:415:
ZoomBubbleView::CloseBubble();
Nit: Put this next to zoom_view_->Update().

[Kyle Horimoto, 2012-08-15 19:29:15]

Put this within a non-NULL check so that it only is called on tab changes.

https://chromiumcodereview.appspot.com/10854169/diff/1/chrome/browser/ui/view...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://chromiumcodereview.appspot.com/10854169/diff/1/chrome/browser/ui/view...
chrome/browser/ui/views/location_bar/location_bar_view.cc:415:
ZoomBubbleView::CloseBubble();
On 2012/08/15 19:19:20, Peter Kasting wrote:
> Nit: Put this next to zoom_view_->Update().

Done.

[commit-bot: I haz the power, 2012-08-15 19:31:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/khorimoto@chromium.org/10854169/1

[commit-bot: I haz the power, 2012-08-15 19:38:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/khorimoto@chromium.org/10854169/4002

[Peter Kasting, 2012-08-15 19:40:28]

https://chromiumcodereview.appspot.com/10854169/diff/4002/chrome/browser/ui/v...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://chromiumcodereview.appspot.com/10854169/diff/4002/chrome/browser/ui/v...
chrome/browser/ui/views/location_bar/location_bar_view.cc:402: // zoom bubble
should be hidden since it represents the zoom on another tab.
So, if we change URLs within the tab, and the new URL and old URL have different
zoom levels, will the bubble stay open but change its displayed value?  If so
that seems a little weird.

https://chromiumcodereview.appspot.com/10854169/diff/4002/chrome/browser/ui/v...
chrome/browser/ui/views/location_bar/location_bar_view.cc:405:
zoom_view_->Update();
Having this call after CloseBubble() doesn't risk re-opening the bubble, does
it?

[commit-bot: I haz the power, 2012-08-15 20:07:09]

Try job failure for 10854169-4002 on mac for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

Step "update" is always a major failure.
Look at the try server FAQ for more details.

[Kyle Horimoto, 2012-08-15 20:30:19]

https://chromiumcodereview.appspot.com/10854169/diff/4002/chrome/browser/ui/v...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://chromiumcodereview.appspot.com/10854169/diff/4002/chrome/browser/ui/v...
chrome/browser/ui/views/location_bar/location_bar_view.cc:402: // zoom bubble
should be hidden since it represents the zoom on another tab.
On 2012/08/15 19:40:28, Peter Kasting wrote:
> So, if we change URLs within the tab, and the new URL and old URL have
different
> zoom levels, will the bubble stay open but change its displayed value?  If so
> that seems a little weird.

Actually, you're right. Probably better to get rid of the check. I tested it
without the check and it correctly gits rid of the bubble.

https://chromiumcodereview.appspot.com/10854169/diff/4002/chrome/browser/ui/v...
chrome/browser/ui/views/location_bar/location_bar_view.cc:405:
zoom_view_->Update();
On 2012/08/15 19:40:28, Peter Kasting wrote:
> Having this call after CloseBubble() doesn't risk re-opening the bubble, does
> it?

Nope!

[commit-bot: I haz the power, 2012-08-15 23:06:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/khorimoto@chromium.org/10854169/12001

[commit-bot: I haz the power, 2012-08-16 03:33:12]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[commit-bot: I haz the power, 2012-08-16 17:58:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/khorimoto@chromium.org/10854169/12001

[commit-bot: I haz the power, 2012-08-16 18:30:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/khorimoto@chromium.org/10854169/12001

[commit-bot: I haz the power, 2012-08-16 20:58:56]

Change committed as 151962