Use a placeholder logo image/label in SearchViewController.

chrome/browser/ui/views/search_view_controller.cc

Index: chrome/browser/ui/views/search_view_controller.cc
diff --git a/chrome/browser/ui/views/search_view_controller.cc b/chrome/browser/ui/views/search_view_controller.cc
index 118d301d6293eb4df8958fd2a0006a9e5648ba3c..292d8fd0b88f54faad4fc8e92d327b635adacbd4 100644
--- a/chrome/browser/ui/views/search_view_controller.cc
+++ b/chrome/browser/ui/views/search_view_controller.cc
@@ -4,6 +4,11 @@
 
 #include "chrome/browser/ui/views/search_view_controller.h"
 
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/search_engines/search_engine_type.h"
+#include "chrome/browser/search_engines/template_url.h"
+#include "chrome/browser/search_engines/template_url_service.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/ui/search/search_model.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
 #include "chrome/browser/ui/search/search_types.h"
@@ -18,6 +23,8 @@
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/canvas.h"
+#include "ui/views/controls/image_view.h"
+#include "ui/views/controls/label.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/layout/layout_manager.h"
@@ -96,29 +103,6 @@ class SearchContainerView : public views::View {
   DISALLOW_COPY_AND_ASSIGN(SearchContainerView);
 };
 
-// FixedSizeLayoutManager ------------------------------------------------------
-
-// LayoutManager that returns a specific preferred size.
-
-class FixedSizeLayoutManager : public views::LayoutManager {
- public:
-  explicit FixedSizeLayoutManager(const gfx::Size& size)
-      : preferred_size_(size) {
-  }
-  virtual ~FixedSizeLayoutManager() {}
-
-  // views::LayoutManager overrides:
-  virtual void Layout(views::View* host) OVERRIDE {}
-  virtual gfx::Size GetPreferredSize(views::View* host) OVERRIDE {
-    return preferred_size_;
-  }
-
- private:
-  const gfx::Size preferred_size_;
-
-  DISALLOW_COPY_AND_ASSIGN(FixedSizeLayoutManager);
-};
-
 // NTPViewBackground -----------------------------------------------------------
 
 // Background for the NTP view.
@@ -236,14 +220,17 @@ void SearchViewController::OmniboxPopupViewParent::ChildPreferredSizeChanged(
 // SearchViewController --------------------------------------------------------
 
 SearchViewController::SearchViewController(
+    content::BrowserContext* browser_context,
     ContentsContainer* contents_container)
-    : contents_container_(contents_container),
+    : browser_context_(browser_context),
+      contents_container_(contents_container),
       location_bar_container_(NULL),
       state_(STATE_NOT_VISIBLE),
       tab_contents_(NULL),
       search_container_(NULL),
       ntp_view_(NULL),
-      logo_view_(NULL),
+      logo_label_(NULL),
+      logo_image_(NULL),
       content_view_(NULL),
       omnibox_popup_view_parent_(NULL) {
   omnibox_popup_view_parent_ = new OmniboxPopupViewParent(this);
@@ -281,7 +268,7 @@ void SearchViewController::StackAtTop() {
   if (search_container_) {
     StackViewsLayerAtTop(search_container_);
     StackViewsLayerAtTop(ntp_view_);
-    StackViewsLayerAtTop(logo_view_);
+    StackViewsLayerAtTop(GetLogoView());
     StackWebViewLayerAtTop(content_view_);
   }
 #else
@@ -386,7 +373,7 @@ void SearchViewController::StartAnimation() {
   }
 
   {
-    ui::Layer* logo_layer = logo_view_->layer();
+    ui::Layer* logo_layer = GetLogoView()->layer();
     ui::ScopedLayerAnimationSettings settings(logo_layer->GetAnimator());
     settings.SetTransitionDuration(
         base::TimeDelta::FromMilliseconds(135 * factor));
@@ -419,13 +406,34 @@ void SearchViewController::CreateViews() {
   ntp_view_->SetPaintToLayer(true);
   ntp_view_->layer()->SetMasksToBounds(true);
 
-  logo_view_ = new views::View;
-  logo_view_->SetLayoutManager(
-      new FixedSizeLayoutManager(gfx::Size(300, 200)));
-  logo_view_->set_background(
-      views::Background::CreateSolidBackground(SK_ColorRED));
-  logo_view_->SetPaintToLayer(true);
-  logo_view_->SetFillsBoundsOpaquely(false);
+  const TemplateURL* default_template_url =

[Peter Kasting, 2012/08/16 03:52:41]

This may be NULL.  You must NULL-check before dereferencing below.

(That is, unless you otherwise guarantee that you never even reach here when
there is no default search provider.  That may be a good idea as it's not clear
to me what you're supposed to do here if this is NULL.  If you do this, DCHECK +
comment here.)

[msw, 2012/08/16 08:29:08]

Done. Checking and having a blank Label with no DSE seems fine for now.

+      TemplateURLServiceFactory::GetForProfile(
+          Profile::FromBrowserContext(browser_context_))->
+              GetDefaultSearchProvider();
+
+#if defined(GOOGLE_CHROME_BUILD)
+  if (default_template_url->prepopulate_id() == SEARCH_ENGINE_GOOGLE) {

[Peter Kasting, 2012/08/16 03:52:41]

prepopulate_id() is not a SearchEngineType.  You need to use
TemplateURLPrepopulateData::GetEngineType().

(It is a total coincidence that this happens to work as written.)

[msw, 2012/08/16 08:29:08]

Done. I'll fix the basis for this code (in SearchProvider) at:
http://codereview.chromium.org/10832338/

+    logo_image_ = new views::ImageView();

[Peter Kasting, 2012/08/16 03:52:41]

Given how you're creating these, it might be better to make the member variables
scoped_ptrs and call set_owned_by_client(true).  This ensures these will be
cleaned up properly regardless of if/how they get added to a view hierarchy. 
This might turn out to be important if for example you end up writing the code
such that if the user changes their DSP, they could somehow end up running
through and creating both a logo and a label.

Really I just get nervous when we create a view on the heap and don't in the
same function guarantee that it's owned by a hierarchy.

[msw, 2012/08/16 08:29:08]

Done.

+    logo_image_->SetImage(ui::ResourceBundle::GetSharedInstance().
+                              GetImageSkiaNamed(IDR_GOOGLE_LOGO_LG));
+    logo_image_->SetPaintToLayer(true);
+    logo_image_->SetFillsBoundsOpaquely(false);
+  }
+#endif
+
+  if (!logo_image_) {
+    DCHECK(!default_template_url->short_name().empty());
+    logo_label_ = new views::Label(default_template_url->short_name());
+    // TODO(msw): Use a transparent background color as a workaround to support
+    // using Labels' view layers via gfx::Canvas::NO_SUBPIXEL_RENDERING.
+    logo_label_->SetBackgroundColor(0x00000000);
+    logo_label_->set_background(
+        views::Background::CreateSolidBackground(SK_ColorRED));
+    logo_label_->SetEnabledColor(SK_ColorRED);
+    logo_label_->SetFont(logo_label_->font().DeriveFont(75, gfx::Font::BOLD));
+    logo_label_->SetPaintToLayer(true);
+    logo_label_->SetFillsBoundsOpaquely(false);
+  }
 
   // Reparent the main web contents view out of |contents_container_| and
   // in to |ntp_view_| below.  Reparent back in destructor.
@@ -433,9 +441,10 @@ void SearchViewController::CreateViews() {
   DCHECK(content_view_);
   contents_container_->SetActive(NULL);
 
+  views::View* logo_view = GetLogoView();
   ntp_view_->SetLayoutManager(
-      new NTPViewLayoutManager(logo_view_, content_view_));
-  ntp_view_->AddChildView(logo_view_);
+      new NTPViewLayoutManager(logo_view, content_view_));
+  ntp_view_->AddChildView(logo_view);
   ntp_view_->AddChildView(content_view_);
 
   search_container_ =
@@ -447,6 +456,12 @@ void SearchViewController::CreateViews() {
   contents_container_->SetOverlay(search_container_);
 }
 
+views::View* SearchViewController::GetLogoView() const {
+  if (logo_image_)

[Peter Kasting, 2012/08/16 03:52:41]

Nit: Shorter:

  return logo_image_ ? logo_image_ : logo_label;

[msw, 2012/08/16 08:29:08]

That'd require a cast (Label/ImageView), which David asked that I avoid.

+    return logo_image_;
+  return logo_label_;
+}
+
 void SearchViewController::DestroyViews() {
   if (!search_container_)
     return;
@@ -468,7 +483,8 @@ void SearchViewController::DestroyViews() {
   delete search_container_;
   search_container_ = NULL;
   ntp_view_ = NULL;
-  logo_view_ = NULL;
+  logo_label_ = NULL;
+  logo_image_ = NULL;
   content_view_ = NULL;
 
   state_ = STATE_NOT_VISIBLE;