InstantExtended: Add js api for custom logo.

InstantExtended: Add js api for custom logo.

Background: When the Instant page is warm and the search provider has a
doodle, we want to show it immediately without first showing the
baked-in logo (the user has already seen the baked-in logo transition
away, and seeing that on every NTP would be annoying.)

This adds an API which promises that the current overlay will respond to
an NTP contextchange with a SetInstantPreviewHeight to show a custom
logo. In this case, the NTP can just rely on Instant to draw the logo.

BUG=142785

[Jered, 2012-10-10 22:09:14]

Please see 11091047 which Rietveld ate up for context.

> Plumbing has_logo through the BrowserWindow interface seems misplaced. It
seems
> like this state belongs in SearchModel or somewhere similar from where the
> search_view_controller can extract the info. Perhaps @dhollowa can suggest the
> right course here?
Yeah, initially I wanted to put it in SearchModel, but it is per-browser, not
per tab. I was confused by the fact that there seems to be a browser search
model as well as tab ones... I was also sort of confused about why
SearchViewController is attached to BrowserView and not to Browser.

> 
> I am also mildly concerned about the IPC bloat (due to all of our searchbox
> changes). @sky, what do you think? If nobody cares, this is fine with me too.
If
> you think we should keep IPC surface small whenever possible, the has_logo bit
> can be communicated back in the existing InstantSupportDetermined IPC.
I have a slight preference for doing it this way so that the state is set
immediately through a method call like other state from js->Chrome in the
searchbox API.

> 
>
http://codereview.chromium.org/11094047/diff/1/chrome/browser/instant/instant...
> File chrome/browser/instant/instant_controller.cc (right):
> 
>
http://codereview.chromium.org/11094047/diff/1/chrome/browser/instant/instant...
> chrome/browser/instant/instant_controller.cc:556: (IsOutOfDate() &&
> !active_tab_is_ntp_))
> Don't do the IsOutOfDate or active_tab_is_ntp_ checks. It's entirely possible
> that the hidden page will finish loading when Instant is in an out-of-date
state
> (for example, the omnibox has lost focus), and the user is on a regular,
non-ntp
> page. We still want to note the fact that it has a doodle.

[samarth, 2012-10-10 22:16:52]

http://codereview.chromium.org/11091050/diff/1/chrome/browser/ui/browser_inst...
File chrome/browser/ui/browser_instant_controller.cc (right):

http://codereview.chromium.org/11091050/diff/1/chrome/browser/ui/browser_inst...
chrome/browser/ui/browser_instant_controller.cc:99:
browser_->window()->SetInstantHasCustomLogo(has_logo);
I agree with Sreeram that plumbing this through BrowserView feels weird.

Using a SearchMode state makes some sense since SearchViewController uses the
state to determine what kind of UI to show. So a NTP_NO_LOGO state might work. 
On the other hand that complicates the state transitions there a lot more. 

Yet another option is to create a new Observer interface for listening to state
changes in the Instant overlay. That seems a bit heavyweight for a single bool
but could make more sense if we need to pass through other state here.

[sreeram, 2012-10-10 22:21:55]

On 2012/10/10 22:09:14, Jered wrote:
> > Plumbing has_logo through the BrowserWindow interface seems misplaced. It
> seems
> > like this state belongs in SearchModel or somewhere similar from where the
> > search_view_controller can extract the info. Perhaps @dhollowa can suggest
the
> > right course here?
> Yeah, initially I wanted to put it in SearchModel, but it is per-browser, not
> per tab. I was confused by the fact that there seems to be a browser search
> model as well as tab ones... I was also sort of confused about why
> SearchViewController is attached to BrowserView and not to Browser.

IIUC, Each tab has a SearchModel, and the browser's SearchModel just has a copy
of the active tab's model (said copy getting updated whenever the active tab
changes, in SearchDelegate::OnTabActivated()).

I think what we should do here is have a field in the SearchModel class that is
populated in the Browser's instance (i.e., Browser::search_model_) by Instant.
Then, we can either:
1. Update each tab's model in the other direction (browser -> tab) when the
active tab changes (in the same SearchDelegate method mentioned above), or
2. In SearchViewController, get hold of the Browser's model through
BrowserView::browser().

> > I am also mildly concerned about the IPC bloat (due to all of our searchbox
> > changes). @sky, what do you think? If nobody cares, this is fine with me
too.
> If
> > you think we should keep IPC surface small whenever possible, the has_logo
bit
> > can be communicated back in the existing InstantSupportDetermined IPC.
> I have a slight preference for doing it this way so that the state is set
> immediately through a method call like other state from js->Chrome in the
> searchbox API.

Okay. Unless somebody else pipes up about IPC bloat, I am fine with it.

[Jered, 2012-10-31 23:32:29]

We're not doing this anymore.

On 2012/10/10 22:21:55, sreeram wrote:
> On 2012/10/10 22:09:14, Jered wrote:
> > > Plumbing has_logo through the BrowserWindow interface seems misplaced. It
> > seems
> > > like this state belongs in SearchModel or somewhere similar from where the
> > > search_view_controller can extract the info. Perhaps @dhollowa can suggest
> the
> > > right course here?
> > Yeah, initially I wanted to put it in SearchModel, but it is per-browser,
not
> > per tab. I was confused by the fact that there seems to be a browser search
> > model as well as tab ones... I was also sort of confused about why
> > SearchViewController is attached to BrowserView and not to Browser.
> 
> IIUC, Each tab has a SearchModel, and the browser's SearchModel just has a
copy
> of the active tab's model (said copy getting updated whenever the active tab
> changes, in SearchDelegate::OnTabActivated()).
> 
> I think what we should do here is have a field in the SearchModel class that
is
> populated in the Browser's instance (i.e., Browser::search_model_) by Instant.
> Then, we can either:
> 1. Update each tab's model in the other direction (browser -> tab) when the
> active tab changes (in the same SearchDelegate method mentioned above), or
> 2. In SearchViewController, get hold of the Browser's model through
> BrowserView::browser().
> 
> > > I am also mildly concerned about the IPC bloat (due to all of our
searchbox
> > > changes). @sky, what do you think? If nobody cares, this is fine with me
> too.
> > If
> > > you think we should keep IPC surface small whenever possible, the has_logo
> bit
> > > can be communicated back in the existing InstantSupportDetermined IPC.
> > I have a slight preference for doing it this way so that the state is set
> > immediately through a method call like other state from js->Chrome in the
> > searchbox API.
> 
> Okay. Unless somebody else pipes up about IPC bloat, I am fine with it.