Expose getSearchTerms to tab_model for Android

Expose getSearchTerms to tab_model for Android

This adds the necessary call to expose getSearchTerms to tab_model
through the toolbar_model and ifdefs out the instant process checks
for iOS and Android since neither uses instant for query extraction.

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

[dhollowa, 2013-03-01 00:35:04]

c/b/ui/search LGTM.

[dhollowa, 2013-03-01 00:35:51]

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
File chrome/browser/ui/search/search.cc (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
chrome/browser/ui/search/search.cc:214: #endif
nit:  // !defined(OS_IOS) && !defined(OS_ANDROID)

[Ted C, 2013-03-01 02:50:21]

lgtm w/ questions.  Nothing blocking for me.

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
File chrome/browser/ui/android/tab_model/tab_model.h (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
chrome/browser/ui/android/tab_model/tab_model.h:62: string16
GetSearchTermsForCurrentUrl();
Hmm...I would still probably say CurrentTab instead of CurrentUrl as that is
more consistent with what the tab model should be doing.

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
File chrome/browser/ui/search/search.cc (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
chrome/browser/ui/search/search.cc:195: #if !defined(OS_IOS) &&
!defined(OS_ANDROID)
Should we add a comment that IOS and Android do not support instant and that is
why we skip this check?  Or is that just implied knowledge?

[sreeram, 2013-03-01 16:48:15]

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
File chrome/browser/ui/android/tab_model/tab_model.cc (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
chrome/browser/ui/android/tab_model/tab_model.cc:104: return
search_terms_data.InstantExtendedEnabledParam();
SearchTermsData::InstantExtendedEnabledParam() checks to see if instant-extended
is enabled (as opposed to just query-extraction being enabled). I was under the
impression that only the latter is enabled for android. Is that not true?

When you try this, do you actually get a non-blank string? (specifically,
"espv=2")?

[rohitrao (ping after 24h), 2013-03-01 16:57:13]

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
File chrome/browser/ui/android/tab_model/tab_model.cc (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
chrome/browser/ui/android/tab_model/tab_model.cc:104: return
search_terms_data.InstantExtendedEnabledParam();
On 2013/03/01 16:48:16, sreeram wrote:
> SearchTermsData::InstantExtendedEnabledParam() checks to see if
instant-extended
> is enabled (as opposed to just query-extraction being enabled). I was under
the
> impression that only the latter is enabled for android. Is that not true?
> 
> When you try this, do you actually get a non-blank string? (specifically,
> "espv=2")?

https://codereview.chromium.org/12319124/ is in-flight and will cause
InstantExtendedEnabledParam() to return a non-blank string.

[rohitrao (ping after 24h), 2013-03-01 16:57:49]

search.cc changes LGTM

[sreeram, 2013-03-01 17:00:41]

lgtm

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
File chrome/browser/ui/android/tab_model/tab_model.cc (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
chrome/browser/ui/android/tab_model/tab_model.cc:104: return
search_terms_data.InstantExtendedEnabledParam();
On 2013/03/01 16:57:13, rohitrao wrote:
> https://codereview.chromium.org/12319124/ is in-flight and will cause
> InstantExtendedEnabledParam() to return a non-blank string.

Oh, right. Sorry I forgot about that CL.

[Yusuf, 2013-03-01 18:58:20]

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
File chrome/browser/ui/android/tab_model/tab_model.h (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/android/...
chrome/browser/ui/android/tab_model/tab_model.h:62: string16
GetSearchTermsForCurrentUrl();
On 2013/03/01 02:50:21, Ted C wrote:
> Hmm...I would still probably say CurrentTab instead of CurrentUrl as that is
> more consistent with what the tab model should be doing.

Done.

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
File chrome/browser/ui/search/search.cc (right):

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
chrome/browser/ui/search/search.cc:195: #if !defined(OS_IOS) &&
!defined(OS_ANDROID)
Added comment.

On 2013/03/01 02:50:21, Ted C wrote:
> Should we add a comment that IOS and Android do not support instant and that
is
> why we skip this check?  Or is that just implied knowledge?

https://codereview.chromium.org/12387026/diff/4001/chrome/browser/ui/search/s...
chrome/browser/ui/search/search.cc:214: #endif
On 2013/03/01 00:35:51, dhollowa wrote:
> nit:  // !defined(OS_IOS) && !defined(OS_ANDROID)

Done.

[Yusuf, 2013-03-07 23:34:14]

The first CL that enables espv=1 for Android has committed on
https://codereview.chromium.org/12319124/. I have rebased to all the recent
changes.

[Ted C, 2013-03-07 23:38:51]

On 2013/03/07 23:34:14, Yusuf wrote:
> The first CL that enables espv=1 for Android has committed on
> https://codereview.chromium.org/12319124/. I have rebased to all the recent
> changes.

Still lgtm

[commit-bot: I haz the power, 2013-03-07 23:55:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/12387026/19002

[commit-bot: I haz the power, 2013-03-07 23:55:43]

Failed to apply patch for chrome/browser/ui/search/search.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/search/search.cc
  Hunk #1 FAILED at 184.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/ui/search/search.cc.rej

Patch:       chrome/browser/ui/search/search.cc
Index: chrome/browser/ui/search/search.cc
diff --git a/chrome/browser/ui/search/search.cc
b/chrome/browser/ui/search/search.cc
index
6544bd1423f326f6311f8b8561ca1cc0404ec09f..344ba24c25242fea3911f9df383c17240a77468b
100644
--- a/chrome/browser/ui/search/search.cc
+++ b/chrome/browser/ui/search/search.cc
@@ -184,10 +184,13 @@ string16 GetSearchTermsImpl(const content::WebContents*
contents,
   // faking search terms in the URL. Random pages can't get into the Instant
   // renderer and scripting doesn't work cross-process, so if the page is in
   // the Instant process, we know it isn't being exploited.
+  // Since iOS and Android doesn't use the instant framework, these checks are
+  // disabled for the two platforms.
   Profile* profile =
Profile::FromBrowserContext(contents->GetBrowserContext());
+#if !defined(OS_IOS) && !defined(OS_ANDROID)
   if (!IsRenderedInInstantProcess(contents, profile))
     return string16();
-
+#endif  // !defined(OS_IOS) && !defined(OS_ANDROID)
   // Check to see if search terms have already been extracted.
   string16 search_terms = GetSearchTermsFromNavigationEntry(entry);
   if (!search_terms.empty())

[commit-bot: I haz the power, 2013-03-08 01:49:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/12387026/31001

[commit-bot: I haz the power, 2013-03-08 01:49:19]

Failed to apply patch for chrome/browser/ui/search/search.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/search/search.cc
  Hunk #1 FAILED at 188.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/ui/search/search.cc.rej

Patch:       chrome/browser/ui/search/search.cc
Index: chrome/browser/ui/search/search.cc
diff --git a/chrome/browser/ui/search/search.cc
b/chrome/browser/ui/search/search.cc
index
967a5935de672e5872dae286174624bbd90102a3..1fce0cd47389911021deff735191d58a635145c8
100644
--- a/chrome/browser/ui/search/search.cc
+++ b/chrome/browser/ui/search/search.cc
@@ -188,10 +188,13 @@ string16 GetSearchTermsImpl(const content::WebContents*
contents,
   // faking search terms in the URL. Random pages can't get into the Instant
   // renderer and scripting doesn't work cross-process, so if the page is in
   // the Instant process, we know it isn't being exploited.
+  // Since iOS and Android doesn't use the instant framework, these checks are
+  // disabled for the two platforms.
   Profile* profile =
Profile::FromBrowserContext(contents->GetBrowserContext());
+#if !defined(OS_IOS) && !defined(OS_ANDROID)
   if (!IsRenderedInInstantProcess(contents, profile))
     return string16();
-
+#endif  // !defined(OS_IOS) && !defined(OS_ANDROID)
   // Check to see if search terms have already been extracted.
   string16 search_terms = GetSearchTermsFromNavigationEntry(entry);
   if (!search_terms.empty())

[commit-bot: I haz the power, 2013-03-08 02:00:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/12387026/35001

[commit-bot: I haz the power, 2013-03-08 02:00:21]

Failed to apply patch for chrome/browser/ui/search/search.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/search/search.cc
  Hunk #1 FAILED at 188.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/ui/search/search.cc.rej

Patch:       chrome/browser/ui/search/search.cc
Index: chrome/browser/ui/search/search.cc
diff --git a/chrome/browser/ui/search/search.cc
b/chrome/browser/ui/search/search.cc
index
967a5935de672e5872dae286174624bbd90102a3..1fce0cd47389911021deff735191d58a635145c8
100644
--- a/chrome/browser/ui/search/search.cc
+++ b/chrome/browser/ui/search/search.cc
@@ -188,10 +188,13 @@ string16 GetSearchTermsImpl(const content::WebContents*
contents,
   // faking search terms in the URL. Random pages can't get into the Instant
   // renderer and scripting doesn't work cross-process, so if the page is in
   // the Instant process, we know it isn't being exploited.
+  // Since iOS and Android doesn't use the instant framework, these checks are
+  // disabled for the two platforms.
   Profile* profile =
Profile::FromBrowserContext(contents->GetBrowserContext());
+#if !defined(OS_IOS) && !defined(OS_ANDROID)
   if (!IsRenderedInInstantProcess(contents, profile))
     return string16();
-
+#endif  // !defined(OS_IOS) && !defined(OS_ANDROID)
   // Check to see if search terms have already been extracted.
   string16 search_terms = GetSearchTermsFromNavigationEntry(entry);
   if (!search_terms.empty())

[commit-bot: I haz the power, 2013-03-08 18:12:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusufo@chromium.org/12387026/38001

[commit-bot: I haz the power, 2013-03-08 22:11:53]

Change committed as 187045