Use a placeholder logo image/label in SearchViewController.

chrome/browser/ui/views/search_view_controller.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/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"
 #include "chrome/browser/ui/search/search_ui.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/browser/ui/views/frame/contents_container.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_container.h"
 #include "chrome/browser/ui/webui/instant_ui.h"
 #include "chrome/common/url_constants.h"
 #include "grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 #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"
 
 #if defined(USE_AURA)
 #include "ui/aura/window.h"
 #endif
 
 namespace {
 
@@ skipping 58 matching lines @@
         chrome::search::kResultsSeparatorColor);
   }
 
  private:
   views::View* ntp_view_;
   views::View* omnibox_popup_view_parent_;
 
   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.
 class NTPViewBackground : public views::Background {
  public:
   NTPViewBackground() {}
   virtual ~NTPViewBackground() {}
 
   // views::Background overrides:
   virtual void Paint(gfx::Canvas* canvas, views::View* view) const OVERRIDE {
@@ skipping 97 matching lines @@
     parent()->Layout();
   if (child_visible_ != child->visible()) {
     child_visible_ = child->visible();
     search_view_controller_->PopupVisibilityChanged();
   }
 }
 
 // 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);
 }
 
 SearchViewController::~SearchViewController() {
   if (search_model())
     search_model()->RemoveObserver(this);
 
   // If the |omnibox_popup_view_| isn't parented, delete it. Otherwise it'll be
@@ skipping 17 matching lines @@
     search_model()->AddObserver(this);
 
   UpdateState();
 }
 
 void SearchViewController::StackAtTop() {
 #if defined(USE_AURA)
   if (search_container_) {
     StackViewsLayerAtTop(search_container_);
     StackViewsLayerAtTop(ntp_view_);
-    StackViewsLayerAtTop(logo_view_);
+    StackViewsLayerAtTop(GetLogoView());
     StackWebViewLayerAtTop(content_view_);
   }
 #else
   NOTIMPLEMENTED();
 #endif
   location_bar_container_->StackAtTop();
 }
 
 void SearchViewController::InstantReady() {
 }
@@ skipping 84 matching lines @@
     settings.AddObserver(this);
     settings.SetTransitionDuration(
         base::TimeDelta::FromMilliseconds(180 * factor));
     settings.SetTweenType(ui::Tween::EASE_IN_OUT);
     gfx::Rect bounds(ntp_layer->bounds());
     bounds.set_height(1);
     ntp_layer->SetBounds(bounds);
   }
 
   {
-    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));
     settings.SetTweenType(ui::Tween::EASE_IN_OUT);
     gfx::Rect bounds(logo_layer->bounds());
     bounds.set_y(bounds.y() - 100);
     logo_layer->SetBounds(bounds);
     logo_layer->SetOpacity(0.0f);
   }
 
@@ skipping 12 matching lines @@
 }
 
 void SearchViewController::CreateViews() {
   DCHECK(!ntp_view_);
 
   ntp_view_ = new views::View;
   ntp_view_->set_background(new NTPViewBackground);
   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.
   content_view_ = contents_container_->active();
   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_ =
       new SearchContainerView(ntp_view_, omnibox_popup_view_parent_);
   search_container_->SetPaintToLayer(true);
   search_container_->SetLayoutManager(new views::FillLayout);
   search_container_->layer()->SetMasksToBounds(true);
 
   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;
 
   // We persist the parent of the omnibox so that we don't have to inject a new
   // parent into ToolbarView.
   omnibox_popup_view_parent_->parent()->RemoveChildView(
       omnibox_popup_view_parent_);
 
   // Restore control/parenting of the web_contents back to the
   // |main_contents_view_|.
   ntp_view_->SetLayoutManager(NULL);
   ntp_view_->RemoveChildView(content_view_);
   if (content_view_->web_contents())
     content_view_->web_contents()->GetNativeView()->layer()->SetOpacity(1.0f);
   contents_container_->SetActive(content_view_);
   contents_container_->SetOverlay(NULL);
 
   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;
 }
 
 void SearchViewController::PopupVisibilityChanged() {
   // Don't do anything while animating if the child is visible. Otherwise we'll
   // prematurely cancel the animation.
   if (state_ != STATE_ANIMATING ||
       !omnibox_popup_view_parent_->is_child_visible()) {
     UpdateState();
   }
 }
 
 chrome::search::SearchModel* SearchViewController::search_model() {
   return tab_contents_ ? tab_contents_->search_tab_helper()->model() : NULL;
 }
 
 content::WebContents* SearchViewController::web_contents() {
   return tab_contents_ ? tab_contents_->web_contents() : NULL;
 }