Converts SearchViewController to use live NTP content instead of placeholder

Converts SearchViewController to use live NTP content instead of placeholder

Converts SearchViewController to use live NTP content using views::WebView
instead of placeholder solid rect views::View.

TEST=Launch CrOS/Aura with --enabled-instant-extended-api, observe interactive "Most Visited" section on NTP
BUG=133529
R=sky@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148438

[dhollowa, 2012-07-16 23:59:49]

Not ready for review...  I'm looking for feedback on initial direction. 
Specifically:
 1. How to get NTP contents to appear on initial page load,
 2. How to get the contents to animate with the other elements on the "overlay".

[sky, 2012-07-17 00:24:09]

One other thing to be aware of. We'll need some way to avoid the
painting a white background for the content_view_ while its loading.
We may need code similar to what SearchTabHelper had.

  -Scott

On Mon, Jul 16, 2012 at 4:59 PM,  <dhollowa@chromium.org> wrote:
> Reviewers: sky,
>
> Message:
> Not ready for review...  I'm looking for feedback on initial direction.
> Specifically:
>  1. How to get NTP contents to appear on initial page load,
>  2. How to get the contents to animate with the other elements on the
> "overlay".
>
> Description:
> Converts SearchViewController to use live NTP content instead of placeholder
>
> Converts SearchViewController to use live NTP content using views::WebView
> instead of placeholder solid rect views::View.
>
> TEST=Launch CrOS/Aura with --enabled-instant-extended-api, observe
> interactive
> "Most Visited" section on NTP
> BUG=133529
> R=sky@chromium.org
>
>
> Please review this at https://chromiumcodereview.appspot.com/10787028/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>   M chrome/browser/ui/views/frame/browser_view.cc
>   M chrome/browser/ui/views/search_view_controller.h
>   M chrome/browser/ui/views/search_view_controller.cc
>
>
> Index: chrome/browser/ui/views/frame/browser_view.cc
> diff --git a/chrome/browser/ui/views/frame/browser_view.cc
> b/chrome/browser/ui/views/frame/browser_view.cc
> index
>
3321477e27e2262c83d7e6b3bbc553367b0e5751..a461e2fdc4178fa9dad40fd9f0862e3839c57408
> 100644
> --- a/chrome/browser/ui/views/frame/browser_view.cc
> +++ b/chrome/browser/ui/views/frame/browser_view.cc
> @@ -1926,7 +1926,8 @@ void BrowserView::Init() {
>    // SearchViewController doesn't work on windows yet.
>  #if defined(USE_AURA)
>    if (chrome::search::IsInstantExtendedAPIEnabled(browser_->profile())) {
> -    search_view_controller_.reset(new SearchViewController(contents_));
> +    search_view_controller_.reset(new
> SearchViewController(browser_->profile(),
> +                                                           contents_));
>      omnibox_popup_view_parent =
>          search_view_controller_->omnibox_popup_view_parent();
>    }
> 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
>
ac457f27e2c7ac86cf0579d09fe9e5720adfa52c..7b3b108dc6f3edfe481231ae85a406749fbd0b5f
> 100644
> --- a/chrome/browser/ui/views/search_view_controller.cc
> +++ b/chrome/browser/ui/views/search_view_controller.cc
> @@ -12,11 +12,13 @@
>  #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/compositor/layer.h"
>  #include "ui/compositor/scoped_layer_animation_settings.h"
>  #include "ui/base/resource/resource_bundle.h"
>  #include "ui/gfx/canvas.h"
> +#include "ui/views/controls/webview/webview.h"
>  #include "ui/views/layout/fill_layout.h"
>  #include "ui/views/layout/layout_manager.h"
>
> @@ -141,12 +143,11 @@ class NTPViewLayoutManager : public
> views::LayoutManager {
>          logo_pref.width(),
>          logo_pref.height());
>
> -    gfx::Size content_pref(content_view_->GetPreferredSize());
> -    int content_y = std::max(chrome::search::kOmniboxYPosition + 50,
> -                             host->height() - content_pref.height() - 50);
> -    content_view_->SetBounds((host->width() - content_pref.width()) / 2,
> -                             content_y, content_pref.width(),
> -                             content_pref.height());
> +    int content_y = chrome::search::kOmniboxYPosition + 50;
> +    content_view_->SetBounds(0,
> +                             content_y,
> +                             host->width(),
> +                             host->height() - content_y);
>    }
>
>    virtual gfx::Size GetPreferredSize(views::View* host) OVERRIDE {
> @@ -216,8 +217,10 @@ 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_(NULL),
> @@ -334,6 +337,7 @@ void SearchViewController::SetState(State state) {
>      case STATE_NTP:
>        DestroyViews();
>        CreateViews();
> +      LoadContentView();
>        break;
>
>      case STATE_ANIMATING:
> @@ -405,17 +409,14 @@ void SearchViewController::CreateViews() {
>    logo_view_->SetPaintToLayer(true);
>    logo_view_->SetFillsBoundsOpaquely(false);
>
> -  // TODO: replace with WebContents for NTP.
> -  content_view_ = new views::View;
> -  content_view_->SetLayoutManager(
> -      new FixedSizeLayoutManager(gfx::Size(400, 200)));
> -  content_view_->set_background(
> -      views::Background::CreateSolidBackground(SK_ColorBLUE));
> +  content_view_ = new views::WebView(browser_context_);
>    content_view_->SetPaintToLayer(true);
>    content_view_->SetFillsBoundsOpaquely(false);
>
>    ntp_view_->SetLayoutManager(
>        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_);
> @@ -423,9 +424,6 @@ void SearchViewController::CreateViews() {
>    search_container_->SetLayoutManager(new views::FillLayout);
>    search_container_->layer()->SetMasksToBounds(true);
>
> -  ntp_view_->AddChildView(logo_view_);
> -  ntp_view_->AddChildView(content_view_);
> -
>    contents_container_->SetOverlay(search_container_);
>  }
>
> @@ -440,8 +438,10 @@ void SearchViewController::DestroyViews() {
>
>    contents_container_->SetOverlay(NULL);
>    delete search_container_;
> -  search_container_ = ntp_view_ = NULL;
> -  content_view_ = logo_view_ = NULL;
> +  search_container_ = NULL;
> +  ntp_view_ = NULL;
> +  logo_view_ = NULL;
> +  content_view_ = NULL;
>
>    state_ = STATE_NOT_VISIBLE;
>  }
> @@ -455,6 +455,13 @@ void SearchViewController::PopupVisibilityChanged() {
>    }
>  }
>
> +void SearchViewController::LoadContentView() {
> +  if (!tab_)
> +    return;
> +
> +  content_view_->LoadInitialURL(GURL(chrome::kChromeUINewTabURL));
> +}
> +
>  chrome::search::SearchModel* SearchViewController::search_model() {
>    return tab_ ? tab_->search_tab_helper()->model() : NULL;
>  }
> Index: chrome/browser/ui/views/search_view_controller.h
> diff --git a/chrome/browser/ui/views/search_view_controller.h
> b/chrome/browser/ui/views/search_view_controller.h
> index
>
17893b3fdcfcd4d069d5f7768f3c8ff6ce4148b7..ee4589a902d2276df8526cdfcfd8b98cff1f58e7
> 100644
> --- a/chrome/browser/ui/views/search_view_controller.h
> +++ b/chrome/browser/ui/views/search_view_controller.h
> @@ -20,8 +20,13 @@ class SearchModel;
>  }
>  }
>
> +namespace content {
> +class BrowserContext;
> +}
> +
>  namespace views {
>  class View;
> +class WebView;
>  }
>
>  // SearchViewController maintains the search overlay (native new tab page).
> @@ -32,7 +37,8 @@ class SearchViewController
>      : public chrome::search::SearchModelObserver,
>        public ui::ImplicitAnimationObserver {
>   public:
> -  explicit SearchViewController(ContentsContainer* contents_container);
> +  explicit SearchViewController(content::BrowserContext* browser_context,
> +                                ContentsContainer* contents_container);
>    virtual ~SearchViewController();
>
>    views::View* omnibox_popup_view_parent();
> @@ -92,6 +98,9 @@ class SearchViewController
>    // Destroys the various views.
>    void DestroyViews();
>
> +  // Loads the new tab page into |content_view_|.
> +  void LoadContentView();
> +
>    // Invoked when the visibility of the omnibox popup changes.
>    void PopupVisibilityChanged();
>
> @@ -99,6 +108,9 @@ class SearchViewController
>    // is no current tab.
>    chrome::search::SearchModel* search_model();
>
> +  // The profile.
> +  content::BrowserContext* browser_context_;
> +
>    // Where the overlay is placed.
>    ContentsContainer* contents_container_;
>
> @@ -141,7 +153,7 @@ class SearchViewController
>    views::View* search_container_;
>    views::View* ntp_view_;
>    views::View* logo_view_;
> -  views::View* content_view_;
> +  views::WebView* content_view_;
>    OmniboxPopupViewParent* omnibox_popup_view_parent_;
>
>    DISALLOW_COPY_AND_ASSIGN(SearchViewController);
>
>

[dhollowa, 2012-07-17 00:46:41]

Yes, I'm definitely seeing the white-blanking prior to the html showing up.

On 2012/07/17 00:24:09, sky wrote:
> One other thing to be aware of. We'll need some way to avoid the
> painting a white background for the content_view_ while its loading.
> We may need code similar to what SearchTabHelper had.
> 
>   -Scott
> 
> On Mon, Jul 16, 2012 at 4:59 PM,  <mailto:dhollowa@chromium.org> wrote:
> > Reviewers: sky,
> >
> > Message:
> > Not ready for review...  I'm looking for feedback on initial direction.
> > Specifically:
> >  1. How to get NTP contents to appear on initial page load,
> >  2. How to get the contents to animate with the other elements on the
> > "overlay".
> >
> > Description:
> > Converts SearchViewController to use live NTP content instead of placeholder
> >
> > Converts SearchViewController to use live NTP content using views::WebView
> > instead of placeholder solid rect views::View.
> >
> > TEST=Launch CrOS/Aura with --enabled-instant-extended-api, observe
> > interactive
> > "Most Visited" section on NTP
> > BUG=133529
> > mailto:R=sky@chromium.org
> >
> >
> > Please review this at https://chromiumcodereview.appspot.com/10787028/
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src
> >
> > Affected files:
> >   M chrome/browser/ui/views/frame/browser_view.cc
> >   M chrome/browser/ui/views/search_view_controller.h
> >   M chrome/browser/ui/views/search_view_controller.cc
> >
> >
> > Index: chrome/browser/ui/views/frame/browser_view.cc
> > diff --git a/chrome/browser/ui/views/frame/browser_view.cc
> > b/chrome/browser/ui/views/frame/browser_view.cc
> > index
> >
>
3321477e27e2262c83d7e6b3bbc553367b0e5751..a461e2fdc4178fa9dad40fd9f0862e3839c57408
> > 100644
> > --- a/chrome/browser/ui/views/frame/browser_view.cc
> > +++ b/chrome/browser/ui/views/frame/browser_view.cc
> > @@ -1926,7 +1926,8 @@ void BrowserView::Init() {
> >    // SearchViewController doesn't work on windows yet.
> >  #if defined(USE_AURA)
> >    if (chrome::search::IsInstantExtendedAPIEnabled(browser_->profile())) {
> > -    search_view_controller_.reset(new SearchViewController(contents_));
> > +    search_view_controller_.reset(new
> > SearchViewController(browser_->profile(),
> > +                                                           contents_));
> >      omnibox_popup_view_parent =
> >          search_view_controller_->omnibox_popup_view_parent();
> >    }
> > 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
> >
>
ac457f27e2c7ac86cf0579d09fe9e5720adfa52c..7b3b108dc6f3edfe481231ae85a406749fbd0b5f
> > 100644
> > --- a/chrome/browser/ui/views/search_view_controller.cc
> > +++ b/chrome/browser/ui/views/search_view_controller.cc
> > @@ -12,11 +12,13 @@
> >  #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/compositor/layer.h"
> >  #include "ui/compositor/scoped_layer_animation_settings.h"
> >  #include "ui/base/resource/resource_bundle.h"
> >  #include "ui/gfx/canvas.h"
> > +#include "ui/views/controls/webview/webview.h"
> >  #include "ui/views/layout/fill_layout.h"
> >  #include "ui/views/layout/layout_manager.h"
> >
> > @@ -141,12 +143,11 @@ class NTPViewLayoutManager : public
> > views::LayoutManager {
> >          logo_pref.width(),
> >          logo_pref.height());
> >
> > -    gfx::Size content_pref(content_view_->GetPreferredSize());
> > -    int content_y = std::max(chrome::search::kOmniboxYPosition + 50,
> > -                             host->height() - content_pref.height() - 50);
> > -    content_view_->SetBounds((host->width() - content_pref.width()) / 2,
> > -                             content_y, content_pref.width(),
> > -                             content_pref.height());
> > +    int content_y = chrome::search::kOmniboxYPosition + 50;
> > +    content_view_->SetBounds(0,
> > +                             content_y,
> > +                             host->width(),
> > +                             host->height() - content_y);
> >    }
> >
> >    virtual gfx::Size GetPreferredSize(views::View* host) OVERRIDE {
> > @@ -216,8 +217,10 @@ 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_(NULL),
> > @@ -334,6 +337,7 @@ void SearchViewController::SetState(State state) {
> >      case STATE_NTP:
> >        DestroyViews();
> >        CreateViews();
> > +      LoadContentView();
> >        break;
> >
> >      case STATE_ANIMATING:
> > @@ -405,17 +409,14 @@ void SearchViewController::CreateViews() {
> >    logo_view_->SetPaintToLayer(true);
> >    logo_view_->SetFillsBoundsOpaquely(false);
> >
> > -  // TODO: replace with WebContents for NTP.
> > -  content_view_ = new views::View;
> > -  content_view_->SetLayoutManager(
> > -      new FixedSizeLayoutManager(gfx::Size(400, 200)));
> > -  content_view_->set_background(
> > -      views::Background::CreateSolidBackground(SK_ColorBLUE));
> > +  content_view_ = new views::WebView(browser_context_);
> >    content_view_->SetPaintToLayer(true);
> >    content_view_->SetFillsBoundsOpaquely(false);
> >
> >    ntp_view_->SetLayoutManager(
> >        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_);
> > @@ -423,9 +424,6 @@ void SearchViewController::CreateViews() {
> >    search_container_->SetLayoutManager(new views::FillLayout);
> >    search_container_->layer()->SetMasksToBounds(true);
> >
> > -  ntp_view_->AddChildView(logo_view_);
> > -  ntp_view_->AddChildView(content_view_);
> > -
> >    contents_container_->SetOverlay(search_container_);
> >  }
> >
> > @@ -440,8 +438,10 @@ void SearchViewController::DestroyViews() {
> >
> >    contents_container_->SetOverlay(NULL);
> >    delete search_container_;
> > -  search_container_ = ntp_view_ = NULL;
> > -  content_view_ = logo_view_ = NULL;
> > +  search_container_ = NULL;
> > +  ntp_view_ = NULL;
> > +  logo_view_ = NULL;
> > +  content_view_ = NULL;
> >
> >    state_ = STATE_NOT_VISIBLE;
> >  }
> > @@ -455,6 +455,13 @@ void SearchViewController::PopupVisibilityChanged() {
> >    }
> >  }
> >
> > +void SearchViewController::LoadContentView() {
> > +  if (!tab_)
> > +    return;
> > +
> > +  content_view_->LoadInitialURL(GURL(chrome::kChromeUINewTabURL));
> > +}
> > +
> >  chrome::search::SearchModel* SearchViewController::search_model() {
> >    return tab_ ? tab_->search_tab_helper()->model() : NULL;
> >  }
> > Index: chrome/browser/ui/views/search_view_controller.h
> > diff --git a/chrome/browser/ui/views/search_view_controller.h
> > b/chrome/browser/ui/views/search_view_controller.h
> > index
> >
>
17893b3fdcfcd4d069d5f7768f3c8ff6ce4148b7..ee4589a902d2276df8526cdfcfd8b98cff1f58e7
> > 100644
> > --- a/chrome/browser/ui/views/search_view_controller.h
> > +++ b/chrome/browser/ui/views/search_view_controller.h
> > @@ -20,8 +20,13 @@ class SearchModel;
> >  }
> >  }
> >
> > +namespace content {
> > +class BrowserContext;
> > +}
> > +
> >  namespace views {
> >  class View;
> > +class WebView;
> >  }
> >
> >  // SearchViewController maintains the search overlay (native new tab page).
> > @@ -32,7 +37,8 @@ class SearchViewController
> >      : public chrome::search::SearchModelObserver,
> >        public ui::ImplicitAnimationObserver {
> >   public:
> > -  explicit SearchViewController(ContentsContainer* contents_container);
> > +  explicit SearchViewController(content::BrowserContext* browser_context,
> > +                                ContentsContainer* contents_container);
> >    virtual ~SearchViewController();
> >
> >    views::View* omnibox_popup_view_parent();
> > @@ -92,6 +98,9 @@ class SearchViewController
> >    // Destroys the various views.
> >    void DestroyViews();
> >
> > +  // Loads the new tab page into |content_view_|.
> > +  void LoadContentView();
> > +
> >    // Invoked when the visibility of the omnibox popup changes.
> >    void PopupVisibilityChanged();
> >
> > @@ -99,6 +108,9 @@ class SearchViewController
> >    // is no current tab.
> >    chrome::search::SearchModel* search_model();
> >
> > +  // The profile.
> > +  content::BrowserContext* browser_context_;
> > +
> >    // Where the overlay is placed.
> >    ContentsContainer* contents_container_;
> >
> > @@ -141,7 +153,7 @@ class SearchViewController
> >    views::View* search_container_;
> >    views::View* ntp_view_;
> >    views::View* logo_view_;
> > -  views::View* content_view_;
> > +  views::WebView* content_view_;
> >    OmniboxPopupViewParent* omnibox_popup_view_parent_;
> >
> >    DISALLOW_COPY_AND_ASSIGN(SearchViewController);
> >
> >

[dhollowa, 2012-07-20 00:34:09]

This edges closer...  In that there's actual NTP content rendering in the
content_view_.  Still yet to come will be:
  - Tab switching back and forth to NTP is currently borked,
  - The white background of the NTP and the flash needs work,
  - This CL creates a new NTP web_contents for each navigation, but this NTP
section really needs to be stateful across navigation, so either we can reuse
the tab_contents' web_contents in the overlay, or create a new web_contents that
hangs off of the tab_contents->search_tab_helper(), or something...

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_tab_helper.cc (right):

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_tab_helper.cc:63: (edit_model->has_focus() &&
edit_model->user_input_in_progress())) {
Until zero-suggest lands, the presence of the suggestions popup dictates whether
we're in SEARCH mode.  Kuan and I have a spreadsheet that clarifies all this. 
We'll be sending it out shortly.

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
File chrome/browser/ui/views/search_view_controller.cc (left):

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
chrome/browser/ui/views/search_view_controller.cc:247: if (tab_ == tab)
I pulled this logic out because this class should really observe the browser's
search_model.

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
File chrome/browser/ui/views/search_view_controller.cc (right):

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
chrome/browser/ui/views/search_view_controller.cc:462: DCHECK_EQ(type,
content::NOTIFICATION_WEB_CONTENTS_CONNECTED);
sky, what I'm seeing is that during launch, when the initial overlay is created
no rendering of the |content_view_->web_contents()| is happening even though
|LoadInitialURL| is getting called above in |SetState|.  So I added in this "one
shot" observer and it "fixes" the issue, in that the web_contents now get
rendered properly.  I don't fully grok this though... wondering if the RVH or
something in Views is not fully initialized during browser launch?

I did notice the instant code has some mysterious calls to |WasHidden| and
|WasRestored|...  Thoughts?

[sky, 2012-07-20 15:58:13]

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
File chrome/browser/ui/views/search_view_controller.cc (left):

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
chrome/browser/ui/views/search_view_controller.cc:247: if (tab_ == tab)
On 2012/07/20 00:34:09, dhollowa wrote:
> I pulled this logic out because this class should really observe the browser's
> search_model.

This code needs to run at a specific time. The ordering can't be guaranteed if
you make this listen to the searchmodel.

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
File chrome/browser/ui/views/search_view_controller.cc (right):

http://codereview.chromium.org/10787028/diff/4001/chrome/browser/ui/views/sea...
chrome/browser/ui/views/search_view_controller.cc:462: DCHECK_EQ(type,
content::NOTIFICATION_WEB_CONTENTS_CONNECTED);
On 2012/07/20 00:34:09, dhollowa wrote:
> sky, what I'm seeing is that during launch, when the initial overlay is
created
> no rendering of the |content_view_->web_contents()| is happening even though
> |LoadInitialURL| is getting called above in |SetState|.  So I added in this
"one
> shot" observer and it "fixes" the issue, in that the web_contents now get
> rendered properly.  I don't fully grok this though... wondering if the RVH or
> something in Views is not fully initialized during browser launch?
> 
> I did notice the instant code has some mysterious calls to |WasHidden| and
> |WasRestored|...  Thoughts?

At the time this class is created everything should be ready, so I'm not sure
why you need to do this. Try seeing if the RenderWidgetHostImpl doesn't think
its still hidden (WasHidden was invoked last).

As to the code in InstantLoader. Would be nice if I had some comments, eh? I
believe I added that so the page would attempt to paint, otherwise it wouldn't.
I think its only needed for the non-search url path, which isn't used anymore.
So, that code is likely dead.

[dhollowa, 2012-07-25 18:50:30]

The issue with stacking turned out to be in View::ReorderChildView() at:

  if (use_acceleration_when_possible)
    ReorderLayers();

For the WebView widget this would mess with the stacking since the widget is up
and out of the normal view hierarchy.  Ideally, a WebView's layer would live in
the normal nested place layer-wise, but that doesn't seem to be viable with
current Views.

This workaround seems like the most robust since layout follows tree mutations.

My next step will be to move the web_contents out into the SearchTabHelper class
so each NTP can hold its own state (on the page).

[dhollowa, 2012-07-25 18:51:51]

To clarify, this is ready for review now.  I'd like to land the move of
web_contents to SearchTabHelper as a subsequent CL.

[sky, 2012-07-25 21:11:53]

LGTM

[commit-bot: I haz the power, 2012-07-25 21:21:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dhollowa@chromium.org/10787028/14001

[commit-bot: I haz the power, 2012-07-25 22:48:42]

Change committed as 148438