Enable prefetch-search-results on Desktop Chrome by default.

Enable prefetch-search-results on Desktop Chrome by default.

BUG=none
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283367

[kmadhusu, 2014-07-08 17:42:04]

jered@: Please review.

Thanks.

[kmadhusu, 2014-07-08 17:42:04]

jered@: Please review.

Thanks.

[Jered, 2014-07-14 16:18:27]

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/search/se...
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/search/se...
chrome/browser/search/search.cc:612: return true;
nit: It would be slightly clearer to write #else ... #endif around this return

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc (right):

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc:130: } else {
Technically, don't you still need to call ShouldPrefetchSearchResults() here? It
only returns true if InstantExtendedAPI is enabled, not always.

[kmadhusu, 2014-07-14 19:37:15]

Addressed comments. PTAL.

Thanks.

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/search/se...
File chrome/browser/search/search.cc (right):

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/search/se...
chrome/browser/search/search.cc:612: return true;
On 2014/07/14 16:18:27, Jered wrote:
> nit: It would be slightly clearer to write #else ... #endif around this return

Done.

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc (right):

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc:130: } else {
On 2014/07/14 16:18:27, Jered wrote:
> Technically, don't you still need to call ShouldPrefetchSearchResults() here?
It
> only returns true if InstantExtendedAPI is enabled, not always.

This function is called only when InstantExtendedAPI is enabled. We have that
check in OmniboxController::OnResultChanged(). Can I add a DCHECK() here instead
of a if condition?

[Jered, 2014-07-14 23:23:56]

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
File chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc (right):

https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc:130: } else {
On 2014/07/14 19:37:15, kmadhusu wrote:
> On 2014/07/14 16:18:27, Jered wrote:
> > Technically, don't you still need to call ShouldPrefetchSearchResults()
here?
> It
> > only returns true if InstantExtendedAPI is enabled, not always.
> 
> This function is called only when InstantExtendedAPI is enabled. We have that
> check in OmniboxController::OnResultChanged(). Can I add a DCHECK() here
instead
> of a if condition? 

Sure, that's fine.

[kmadhusu, 2014-07-14 23:48:33]

On 2014/07/14 23:23:56, Jered wrote:
>
https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
> File chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc (right):
> 
>
https://codereview.chromium.org/360373006/diff/70001/chrome/browser/ui/omnibo...
> chrome/browser/ui/omnibox/omnibox_current_page_delegate_impl.cc:130: } else {
> On 2014/07/14 19:37:15, kmadhusu wrote:
> > On 2014/07/14 16:18:27, Jered wrote:
> > > Technically, don't you still need to call ShouldPrefetchSearchResults()
> here?
> > It
> > > only returns true if InstantExtendedAPI is enabled, not always.
> > 
> > This function is called only when InstantExtendedAPI is enabled. We have
that
> > check in OmniboxController::OnResultChanged(). Can I add a DCHECK() here
> instead
> > of a if condition? 
> 
> Sure, that's fine.

Done. Added DCHECK. PTAL. 

Thanks.

[kmadhusu, 2014-07-15 18:19:01]

pkasting@: Please review omnibox_current_page_delegate_impl.cc changes.

Thanks.

[Peter Kasting, 2014-07-15 18:28:51]

LGTM

[Jered, 2014-07-15 21:35:12]

lgtm

[kmadhusu, 2014-07-15 21:52:31]

The CQ bit was checked by kmadhusu@chromium.org

[commit-bot: I haz the power, 2014-07-15 21:54:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kmadhusu@chromium.org/360373006/130001

[commit-bot: I haz the power, 2014-07-16 05:55:34]

Change committed as 283367