Clicks broken in NTP section of Search overlay

Clicks broken in NTP section of Search overlay

This removes prior solution where a duplicate web_contents for the NTP was
stored in the search_tab_helper.  Replaces that solution with this where the
main browser window's web view is reparented into the search overlay when the
search overlay is active and displaying the NTP.  When the NTP is dismissed or
navigated away, the web view is reparented back to the main browser_view's
contents container as per usual.

BUG=133529
TEST=--enable-instant-extended-api, load NTP, click on a most visited link.  Expect navigation in main browser window.
R=sky@chromium.org


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

[sky, 2012-08-09 00:39:14]

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/sear...
File chrome/browser/ui/search/search_tab_helper.h (right):

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/sear...
chrome/browser/ui/search/search_tab_helper.h:9: #include
"base/memory/scoped_ptr.h"
nit: remove this include now.

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/sear...
chrome/browser/ui/search/search_tab_helper.h:19: class WebContents;
nit: I don't think you need this anymore.

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/view...
File chrome/browser/ui/views/search_view_controller.cc (right):

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/view...
chrome/browser/ui/views/search_view_controller.cc:260: }
Are there any timing issues here if content_view_ has been reparented? By that I
mean I believe this could change the order the content_view_ is destroyed in. I
don't know if that'll cause problems.

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/view...
chrome/browser/ui/views/search_view_controller.cc:432:
contents_container_->SetActive(NULL);
How about a DCHECK(content_view_) between 431/432.

[dhollowa, 2012-08-09 16:07:25]

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/sear...
File chrome/browser/ui/search/search_tab_helper.h (right):

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/sear...
chrome/browser/ui/search/search_tab_helper.h:9: #include
"base/memory/scoped_ptr.h"
On 2012/08/09 00:39:14, sky wrote:
> nit: remove this include now.

Done.

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/sear...
chrome/browser/ui/search/search_tab_helper.h:19: class WebContents;
On 2012/08/09 00:39:14, sky wrote:
> nit: I don't think you need this anymore.

Done.

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/view...
File chrome/browser/ui/views/search_view_controller.cc (right):

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/view...
chrome/browser/ui/views/search_view_controller.cc:260: }
On 2012/08/09 00:39:14, sky wrote:
> Are there any timing issues here if content_view_ has been reparented? By that
I
> mean I believe this could change the order the content_view_ is destroyed in.
I
> don't know if that'll cause problems.

It is certainly true that this has changed the destruction order of the
content_view_.  I'm not seeing any adverse effects after playing around with
different scenarios.  And from my read of ~BrowserView() the shutdown
dependencies are  on profile and browser, not on the content_view_.  Also, the
web_contents is not owned by the content_view_, so I think this is ok.

https://chromiumcodereview.appspot.com/10832216/diff/1/chrome/browser/ui/view...
chrome/browser/ui/views/search_view_controller.cc:432:
contents_container_->SetActive(NULL);
On 2012/08/09 00:39:14, sky wrote:
> How about a DCHECK(content_view_) between 431/432.

Done.

[sky, 2012-08-09 16:35:21]

LGTM

[commit-bot: I haz the power, 2012-08-09 16:44:10]

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

[commit-bot: I haz the power, 2012-08-09 18:06:04]

Change committed as 150849