Add title to current page in zero suggest.

components/omnibox/browser/verbatim_match.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/omnibox/browser/verbatim_match.h"
 
+#include "base/feature_list.h"
 #include "base/logging.h"
 #include "components/omnibox/browser/autocomplete_classifier.h"
 #include "components/omnibox/browser/autocomplete_input.h"
 #include "components/omnibox/browser/autocomplete_provider_client.h"
 #include "components/omnibox/browser/history_url_provider.h"
 #include "url/gurl.h"
 
 AutocompleteMatch VerbatimMatchForURL(
     AutocompleteProviderClient* client,
     const AutocompleteInput& input,
@@ skipping 15 matching lines @@
   } else {
     client->Classify(input.text(), false, true,
                      input.current_page_classification(), &match, nullptr);
   }
   match.allowed_to_be_default_match = true;
   // The default relevance to use for relevance match. Should be greater than
   // all relevance matches returned by the ZeroSuggest server.
   const int kDefaultVerbatimRelevance = 1300;
   match.relevance =
       verbatim_relevance >= 0 ? verbatim_relevance : kDefaultVerbatimRelevance;
+  if (base::FeatureList::IsEnabled(omnibox::kDisplayTitleForCurrentUrl)) {

[Mark P, 2017/04/12 17:58:46]

nit: include-what-you-use (omnibox_field_trial.h defines this const)

[gcomanici, 2017/04/12 19:25:00]

I couldn't find it there. Could you be more specific? 

[Mark P, 2017/04/12 19:38:01]

Please include components/omnibox/browser/omnibox_field_trial.h here.

kDisplayTitleForCurrentUrl is a constant that's defined there.  The only reason
this code currently compiles is because you're getting it via a transitive
include somewhere.

[gcomanici, 2017/04/12 20:48:22]

Done.

+    match.description = input.current_title();

[Mark P, 2017/04/12 17:58:46]

|destination_url| does not necessarily correspond to input.current_url or
input.current_title.  Please add a new variable into VerbatimMatchForURL().

[gcomanici, 2017/04/12 19:25:00]

I made a change based on your suggestion but without adding a new variable. What
do you mean by `a new variable`?

[Mark P, 2017/04/12 19:38:01]

No, we want the description to be the title. That's the whole point of the
changelist. :-P

But VerbatimMatchForURL() may be called for example like
VerbatimMatchForURL(... input{current_url="google.com", current_title="Google"},
destination_url="irs.gov")
and the code as of the patchset I'm commenting on will create a irs.gov match
with the title Google.  That's not right.

[gcomanici, 2017/04/12 20:48:22]

Got it. Added a check on destination_url.

[Mark P, 2017/04/12 21:16:57]

I was more thinking that you have callers pass in a |title|, just like they do a
|destination_url|.  I think that makes more sense than having this weird side
effect of setting the title only if the destination URL happens to be the same
as the current page.  (VerbatimMatchForURL() isn't called in that many places,
so it's relatively easy to make this change than has better semantics.)

[gcomanici, 2017/04/13 16:38:38]

Makes sense. Done.

+    AutocompleteMatch::ClassifyLocationInString(
+        base::string16::npos, 0, match.description.length(),
+        ACMatchClassification::NONE, &match.description_class);
+  }
   return match;
 }