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

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

+// 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_TOOLBAR_SITE_CHIP_VIEW_H_
+#define CHROME_BROWSER_UI_VIEWS_TOOLBAR_SITE_CHIP_VIEW_H_
+
+#include "base/metrics/field_trial.h"
+#include "chrome/browser/ui/toolbar/toolbar_model.h"
+#include "chrome/browser/ui/views/toolbar/toolbar_button.h"
+#include "ui/views/controls/button/button.h"
+#include "ui/views/controls/button/label_button.h"
+#include "ui/views/view.h"

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

Nit: Almost none of these #includes seem necessary.  Eliminate the ones you
don't need.

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

Done.

+
+class LocationIconView;
+class ToolbarView;
+
+namespace content {
+class WebContents;
+}
+
+namespace views {
+class Label;
+}
+
+class SiteChipView : public ToolbarButton {
+ public:
+  explicit SiteChipView(ToolbarView* toolbar_view);
+  virtual ~SiteChipView();
+
+  void Init();
+
+  void Update(content::WebContents* tab);
+
+  void OnChanged();

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

This doesn't seem to be called, nor do the two functions below.  It's harder to
know what to say about them in such a case than if you added them later in the
context of code that actually called them.

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

Yeah, I'll get rid of them here.

+
+  views::ImageView* GetLocationIconView();
+  const views::ImageView* GetLocationIconView() const;

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

It seems like these should be inlined unix_hacker()-style accessors.

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

They're the same names as the virtuals in the location bar (for when the
experimental location bar will want to delegate bubble anchoring here).

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

Sure, but why does that matter?  It doesn't seem like it adds any additional
readability.

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

ok, I'll inline them

+
+  // views::ButtonListener

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

Nit: You don't inherit directly from views::ButtonListener (or views::View,
below).  Give your actual parent class, then declare overrides in the order
they'd actually appear in the parent, assuming we listed the methods declared in
base classes before those in subclasses.

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

Done.

+  void ButtonPressed(views::Button* sender, const ui::Event& event) OVERRIDE;
+
+  // views::View
+  virtual gfx::Size GetPreferredSize() OVERRIDE;
+  virtual void Layout() OVERRIDE;
+
+ private:
+  ToolbarView* toolbar_view_;
+  views::Label* host_label_;
+  LocationIconView* location_icon_view_;
+
+  DISALLOW_COPY_AND_ASSIGN(SiteChipView);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_TOOLBAR_SITE_CHIP_VIEW_H_