Implements the "Set to default" button on the zoom bubble.

chrome/browser/ui/views/location_bar/location_bar_view.cc

 // Copyright (c) 2012 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/ui/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/stl_util.h"
@@ skipping 77 matching lines @@
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #endif
 
 using content::WebContents;
 using views::View;
 
 namespace {
 
 WebContents* GetWebContentsFromDelegate(LocationBarView::Delegate* delegate) {
   const TabContents* tab_contents = delegate->GetTabContents();

[Peter Kasting, 2012/07/18 03:26:30]

Nit: While you're touching this file, can you maybe make it so we're consistent
about using either GetTabContents() or delegate_->GetTabContents()?

[Kyle Horimoto, 2012/07/18 23:49:38]

Done. But FYI, this specific case is not a member function of LocationBarView,
so we shouldn't be doing that here.

   return tab_contents ? tab_contents->web_contents() : NULL;
 }
 
 Browser* GetBrowserFromDelegate(LocationBarView::Delegate* delegate) {
   WebContents* contents = GetWebContentsFromDelegate(delegate);
   return browser::FindBrowserWithWebContents(contents);
 }
 
 // Height of the location bar's round corner region.
 const int kBorderRoundCornerHeight = 5;
@@ skipping 154 matching lines @@
   keyword_hint_view_->SetFont(font_);
 
   for (int i = 0; i < CONTENT_SETTINGS_NUM_TYPES; ++i) {
     ContentSettingImageView* content_blocked_view =
         new ContentSettingImageView(static_cast<ContentSettingsType>(i), this);
     content_setting_views_.push_back(content_blocked_view);
     AddChildView(content_blocked_view);
     content_blocked_view->SetVisible(false);
   }
 
-  zoom_view_ = new ZoomView(model_);
+  // Create the zoom view. At this point, |delegate_| has not yet been assigned
+  // a TabContents, so calling GetTabContents() will just return NULL. Thus,
+  // |delegate_| itself is passed here, where it can be queried at a later time
+  // for the appropriate TabContents.

[Peter Kasting, 2012/07/18 03:26:30]

Nit: This seems a little sketchy since apparently this "assigning" action
happens completely outside our control or visibility -- later we just happily
call GetWebContentsFromDelegate() on the delegate as well as
delegate_->GetTabContents().  Is it possible to make the tab contents be
non-NULL at the time the delegate is passed in to this object so there isn't a
window like this?  If not, is there a specific time where we know the tab
contents will exist so we can e.g. DCHECK that in GetTabContents() and refer to
it in the comments here?

[Kyle Horimoto, 2012/07/18 23:49:38]

Hmm, not exactly. ToolbarView creates a LocationBarView when the window is
created. At this point, we have only created the container (window) for a
TabContents, not the actual TabContents (which is why we can't just call
GetTabContents() here). After the window is set up,
TabStripModel::AppendTabContents() is called with the current tab contents
(e.g., the website being viewed or the NTP). At this point, |delegate_| (which
is the ToolbarView that created the LocationBarView) can get a reference to the
TabContents. Unfortunately, there doesn't seem to be a way to check this except
to DCHECK() that the pointer returned by GetTabContents() is non-NULL :/. Should
I just do that?

+  zoom_view_ = new ZoomView(model_, delegate_);
   AddChildView(zoom_view_);
 
   if (extensions::switch_utils::IsActionBoxEnabled()) {
     action_box_button_view_ = new ActionBoxButtonView(
         extensions::ExtensionSystem::Get(profile_)->extension_service());
     AddChildView(action_box_button_view_);
   } else if (browser_defaults::bookmarks_enabled && (mode_ == NORMAL)) {
     // Note: condition above means that the star and ChromeToMobile icons are
     // hidden in popups and in the app launcher.
     star_view_ = new StarView(command_updater_);
@@ skipping 223 matching lines @@
 }
 
 void LocationBarView::SetZoomIconState(
     ZoomController::ZoomIconState zoom_icon_state) {
   zoom_view_->SetZoomIconState(zoom_icon_state);
 
   Layout();
   SchedulePaint();
 }
 
 void LocationBarView::ShowZoomBubble(int zoom_percent) {

[Peter Kasting, 2012/07/18 03:26:30]

Nit: If this argument is no longer used, can we un-plumb it?

[Kyle Horimoto, 2012/07/18 23:49:38]

This is already being done: http://codereview.chromium.org/10736028/.

-  ZoomBubbleView::ShowBubble(zoom_view_, zoom_percent, true);
+  ZoomBubbleView::ShowBubble(zoom_view_, GetTabContents(), true);
 }
 
 void LocationBarView::ShowChromeToMobileBubble() {
   Browser* browser = GetBrowserFromDelegate(delegate_);
   chrome::ShowChromeToMobileBubbleView(chrome_to_mobile_view_, browser);
 }
 
 gfx::Point LocationBarView::GetLocationEntryOrigin() const {
   gfx::Point origin(location_entry_view_->bounds().origin());
   // If the UI layout is RTL, the coordinate system is not transformed and
@@ skipping 966 matching lines @@
 }
 
 void LocationBarView::CleanupFadeAnimation() {
   // Since we're no longer animating we don't need our layer.
   SetPaintToLayer(false);
   // Bubble labels don't need a transparent background anymore.
   ev_bubble_view_->SetLabelBackgroundColor(SK_ColorWHITE);
   selected_keyword_view_->SetLabelBackgroundColor(SK_ColorWHITE);
 }
 #endif  // USE_AURA