Adds selection expansion support for Contextual Search.

chrome/browser/android/contextualsearch/contextual_search_delegate.cc

Index: chrome/browser/android/contextualsearch/contextual_search_delegate.cc
diff --git a/chrome/browser/android/contextualsearch/contextual_search_delegate.cc b/chrome/browser/android/contextualsearch/contextual_search_delegate.cc
index dd465e4f837b68e083f669c8fbfafbad32eee19c..7dccc390b9c3946874c0050d56ef160161724a80 100644
--- a/chrome/browser/android/contextualsearch/contextual_search_delegate.cc
+++ b/chrome/browser/android/contextualsearch/contextual_search_delegate.cc
@@ -41,6 +41,7 @@ const char kContextualSearchResponseSelectedTextParam[] = "selected_text";
 const char kContextualSearchResponseSearchTermParam[] = "search_term";
 const char kContextualSearchResponseResolvedTermParam[] = "resolved_term";
 const char kContextualSearchPreventPreload[] = "prevent_preload";
+const char kContextualSearchMentions[] = "mentions";
 const char kContextualSearchServerEndpoint[] = "_/contextualsearch?";
 const int kContextualSearchRequestVersion = 2;
 const char kContextualSearchResolverUrl[] =
@@ -142,6 +143,10 @@ void ContextualSearchDelegate::OnURLFetchComplete(
   std::string display_text;
   std::string alternate_term;
   std::string prevent_preload;
+  size_t mention_start = 0;
+  size_t mention_end = 0;
+  int start_adjust = 0;
+  int end_adjust = 0;
 
   if (source->GetStatus().is_success() && response_code == 200) {
     std::string response;
@@ -149,13 +154,23 @@ void ContextualSearchDelegate::OnURLFetchComplete(
     DCHECK(has_string_response);
     if (has_string_response) {
       DecodeSearchTermsFromJsonResponse(response, &search_term, &display_text,
-                                        &alternate_term, &prevent_preload);
+                                        &alternate_term, &prevent_preload,
+                                        &mention_start, &mention_end);
+      if (mention_start != 0 || mention_end != 0) {
+        start_adjust = context_->start_offset - mention_start;
+        end_adjust = mention_end - context_->end_offset;

[palmer, 2015/07/07 18:39:59]

I'm finding the implications of the implicit type conversions here to be
non-obvious. I'm wondering if perhaps |start_adjust| and |end_adjust| should
also be declared size_t, and then enforce sane ranges here (e.g. [0, 2**20) ).
Does that make sense?

[aurimas (slooooooooow), 2015/07/09 00:20:50]

I added limits to make sure selection is non-zero and it is under 100 characters
as such selections would dismiss contextual search bar anyway.

+
+        // TODO(aurimas): look into why sometimes we try to adjust by -1.
+        // See crbug.com/506381
+        start_adjust = std::max(0, start_adjust);
+        end_adjust = std::max(0, end_adjust);
+      }
     }
   }
   bool is_invalid = response_code == net::URLFetcher::RESPONSE_CODE_INVALID;
   search_term_callback_.Run(
       is_invalid, response_code, search_term, display_text, alternate_term,
-      prevent_preload == kDoPreventPreloadValue);
+      prevent_preload == kDoPreventPreloadValue, start_adjust, end_adjust);
 
   // The ContextualSearchContext is consumed once the request has completed.
   context_.reset();
@@ -399,7 +414,9 @@ void ContextualSearchDelegate::DecodeSearchTermsFromJsonResponse(
     std::string* search_term,
     std::string* display_text,
     std::string* alternate_term,
-    std::string* prevent_preload) {
+    std::string* prevent_preload,
+    size_t* mention_start,
+    size_t* mention_end) {
   bool contains_xssi_escape = response.find(kXssiEscape) == 0;
   const std::string& proper_json =
       contains_xssi_escape ? response.substr(strlen(kXssiEscape)) : response;
@@ -416,6 +433,11 @@ void ContextualSearchDelegate::DecodeSearchTermsFromJsonResponse(
                          display_text)) {
       *display_text = *search_term;
     }
+    // Extract mentions for selection expansion.
+    base::ListValue* mentions_list;
+    dict->GetList(kContextualSearchMentions, &mentions_list);
+    if (mentions_list != NULL && mentions_list->GetSize() >= 2)
+      ExtractMentionsStartEnd(*mentions_list, mention_start, mention_end);
     // If either the selected text or the resolved term is not the search term,
     // use it as the alternate term.
     std::string selected_text;
@@ -445,6 +467,19 @@ int ContextualSearchDelegate::GetSearchTermSurroundingSize() {
   return kContextualSearchDefaultContentSize;
 }
 
+// Extract the Start/End of the mentions in the surrounding text
+// for selection-expansion.
+void ContextualSearchDelegate::ExtractMentionsStartEnd(
+    const base::ListValue& mentions_list,
+    size_t* startResult,
+    size_t* endResult) {
+  int int_value;
+  if (mentions_list.GetInteger(0, &int_value))
+    *startResult = int_value;

[palmer, 2015/07/07 18:39:59]

More strange type-cast semantics. It's OK if the ultimate consumer of these
values protects itself from strange results (as described above). But it looks
odd even so. Maybe a comment to the effect that the (specifically named)
ultimate consumer does protect itself?

[aurimas (slooooooooow), 2015/07/09 00:20:50]

It now makes it 0 or larger.

+  if (mentions_list.GetInteger(1, &int_value))
+    *endResult = int_value;
+}
+
 // Returns the size of the surroundings to be sent to Icing.
 int ContextualSearchDelegate::GetIcingSurroundingSize() {
   std::string param_string = variations::GetVariationParamValue(