[SiteChip] Add the basic painting of the site chip button

chrome/browser/ui/views/toolbar/site_chip_view.cc

+// Copyright (c) 2013 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/toolbar/site_chip_view.h"
+
+#include "base/prefs/pref_service.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search/search.h"
+#include "chrome/browser/themes/theme_properties.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/omnibox/omnibox_view.h"
+#include "chrome/browser/ui/toolbar/toolbar_model.h"
+#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+#include "chrome/browser/ui/views/location_bar/location_icon_view.h"
+#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "chrome/common/pref_names.h"
+#include "content/public/browser/web_contents.h"
+#include "grit/theme_resources.h"
+#include "net/base/net_util.h"
+#include "ui/base/theme_provider.h"
+#include "ui/views/controls/button/label_button_border.h"
+#include "ui/views/controls/label.h"
+
+SiteChipView::SiteChipView(ToolbarView* toolbar_view)
+    : ToolbarButton(this, NULL),
+      toolbar_view_(toolbar_view) {}

[Peter Kasting, 2013/11/19 02:47:32]

Nit: I slightly prefer linebreaking between braces (2 places)

[Greg Billock, 2013/11/20 01:27:35]

Done.

+
+SiteChipView::~SiteChipView() {}
+
+void SiteChipView::ButtonPressed(views::Button* sender,
+                                 const ui::Event& event) {
+  toolbar_view_->location_bar()->GetLocationEntry()->SetFocus();

[Peter Kasting, 2013/11/19 02:47:32]

The fact that all these calls are on the OmniboxView suggests that either that
or the LocationBarView would be a better ButtonListener than |this|.  I suggest
the LcoationBarView, which is already a button listener for various things.

[Greg Billock, 2013/11/20 01:27:35]

Yeah. This is mostly to isolate the experimental code as much as possible.

[Peter Kasting, 2013/11/20 01:49:13]

FWIW, I wouldn't worry about that in principle.  If it's cleaner to put a small
block of code in the LocationBarView, let's just do that.

In this case, it's probably not much of a win, so it doesn't matter much what
you do.  It was more important when ToolbarButton itself was being made a
ButtonListener, which would have been nice to avoid.

[Greg Billock, 2013/11/21 19:29:34]

ack. I'll either move this later or if it grows hair, keep it here. I can't
think now why it'd need to be here, though.

+  toolbar_view_->location_bar()->GetLocationEntry()->SelectAll(true);
+  toolbar_view_->location_bar()->GetLocationEntry()->model()->
+      SetCaretVisibility(true);
+}
+
+void SiteChipView::Init() {
+  ToolbarButton::Init();
+
+  // TODO(gbillock): Would be nice to just use stock LabelButton stuff here.

[Peter Kasting, 2013/11/19 02:47:32]

Why can't you?

[Greg Billock, 2013/11/20 01:27:35]

Need to catch up with other changes -- making the LabelButton image accessible
was the crucial gap, and I did that for the base class.

+  location_icon_view_ = new LocationIconView(toolbar_view_->location_bar());
+  AddChildView(location_icon_view_);
+
+  host_label_ = new views::Label();
+  AddChildView(host_label_);
+
+  // temporary icon filler
+  location_icon_view_->SetImage(GetThemeProvider()->GetImageSkiaNamed(
+      IDR_LOCATION_BAR_HTTP));
+  location_icon_view_->ShowTooltip(true);
+
+  // temporary filler text.
+  host_label_->SetText(ASCIIToUTF16("Site Chip"));
+}
+
+void SiteChipView::Update(content::WebContents* tab) {
+  Layout();
+  SchedulePaint();
+}
+
+void SiteChipView::OnChanged() {
+  Update(toolbar_view_->GetWebContents());
+}
+
+views::ImageView* SiteChipView::GetLocationIconView() {
+  return location_icon_view_;
+}
+
+const views::ImageView* SiteChipView::GetLocationIconView() const {
+  return location_icon_view_;
+}
+
+// Margin padding at the edges of the site chip are
+// ToolbarView::kStandardSpacing.
+// Item spacing within the chip are LocationBarView::GetItemPadding.

[Peter Kasting, 2013/11/19 02:47:32]

This comment doesn't add anything to the code.

[Greg Billock, 2013/11/20 01:27:35]

Yeah, this was scaffolding. :-)

+gfx::Size SiteChipView::GetPreferredSize() {
+  gfx::Size label_size = host_label_->GetPreferredSize();
+  gfx::Size icon_size = location_icon_view_->GetPreferredSize();
+
+  // Add on two spacing widths before and aft, as well as one
+  // between icon and label.
+  gfx::Size size =

[Peter Kasting, 2013/11/19 02:47:32]

Nit: Don't do "Size size = Size(...)", just do "Size size(...)"

[Greg Billock, 2013/11/20 01:27:35]

Switched up.

+      gfx::Size(icon_size.width() + label_size.width() +
+                    2*ToolbarView::kStandardSpacing +

[Peter Kasting, 2013/11/19 02:47:32]

Nit: Spaces around operators (2 places)

Decide whether you're doing "x * 2" or "2 * x" and stick with it.

[Greg Billock, 2013/11/20 01:27:35]

Done.

+                    LocationBarView::GetItemPadding(),
+                icon_size.height());
+
+  return size;
+}
+
+void SiteChipView::Layout() {
+  location_icon_view_->SetBounds(
+      ToolbarView::kStandardSpacing,
+      LocationBarView::kNormalEdgeThickness,
+      location_icon_view_->GetPreferredSize().width(),
+      height() - LocationBarView::kNormalEdgeThickness*2);
+
+  int host_label_x = ToolbarView::kStandardSpacing +
+      location_icon_view_->GetPreferredSize().width() +
+      LocationBarView::GetItemPadding();
+  int host_label_width = width() - host_label_x - ToolbarView::kStandardSpacing;
+  host_label_->SetBounds(host_label_x, LocationBarView::kNormalEdgeThickness,
+      host_label_width, height() - LocationBarView::kNormalEdgeThickness*2);

[Peter Kasting, 2013/11/19 02:47:32]

Nit: All lines of args should begin at the same horizontal location

[Greg Billock, 2013/11/20 01:27:35]

Done.

+}
+