Commit eb0a66c4 authored by Travis Skare's avatar Travis Skare Committed by Commit Bot

[omnibox] Drive: Use server scoring param when provided.

Use full URL through redirect for navigation.

BUG=875535

Change-Id: Ia2a14fb50023dd58b87c562210ed635504d0ef37
Reviewed-on: https://chromium-review.googlesource.com/1176203
Commit-Queue: Travis Skare <skare@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584266}
parent 5a785596
...@@ -276,7 +276,7 @@ bool DocumentProvider::ParseDocumentSearchResults(const base::Value& root_val, ...@@ -276,7 +276,7 @@ bool DocumentProvider::ParseDocumentSearchResults(const base::Value& root_val,
size_t num_results = results_list->GetSize(); size_t num_results = results_list->GetSize();
UMA_HISTOGRAM_COUNTS("Omnibox.DocumentSuggest.ResultCount", num_results); UMA_HISTOGRAM_COUNTS("Omnibox.DocumentSuggest.ResultCount", num_results);
// Create a synthetic score. Eventually we'll have signals from the API. // Create a synthetic score, for when there's no signal from the API.
// For now, allow setting of each of three scores from Finch. // For now, allow setting of each of three scores from Finch.
int score0 = base::GetFieldTrialParamByFeatureAsInt( int score0 = base::GetFieldTrialParamByFeatureAsInt(
omnibox::kDocumentProvider, "DocumentScoreResult1", 1100); omnibox::kDocumentProvider, "DocumentScoreResult1", 1100);
...@@ -316,11 +316,20 @@ bool DocumentProvider::ParseDocumentSearchResults(const base::Value& root_val, ...@@ -316,11 +316,20 @@ bool DocumentProvider::ParseDocumentSearchResults(const base::Value& root_val,
default: default:
break; break;
} }
int server_score;
if (result->GetInteger("score", &server_score)) {
relevance = server_score;
}
AutocompleteMatch match(this, relevance, false, AutocompleteMatch match(this, relevance, false,
AutocompleteMatchType::DOCUMENT_SUGGESTION); AutocompleteMatchType::DOCUMENT_SUGGESTION);
// Use full URL for displayed text and navigation. Use "originalUrl" for
// deduping if present.
match.fill_into_edit = url;
match.destination_url = GURL(url);
base::string16 original_url; base::string16 original_url;
result->GetString("originalUrl", &original_url); // optional. if (result->GetString("originalUrl", &original_url)) {
match.destination_url = GURL(!original_url.empty() ? original_url : url); match.stripped_destination_url = GURL(original_url);
}
match.contents = AutocompleteMatch::SanitizeString(title); match.contents = AutocompleteMatch::SanitizeString(title);
AutocompleteMatch::AddLastClassificationIfNecessary( AutocompleteMatch::AddLastClassificationIfNecessary(
&match.contents_class, 0, ACMatchClassification::NONE); &match.contents_class, 0, ACMatchClassification::NONE);
......
...@@ -190,7 +190,9 @@ TEST_F(DocumentProviderTest, ParseDocumentSearchResults) { ...@@ -190,7 +190,9 @@ TEST_F(DocumentProviderTest, ParseDocumentSearchResults) {
"results": [ "results": [
{ {
"title": "Document 1", "title": "Document 1",
"url": "https://documentprovider.tld/doc?id=1" "url": "https://documentprovider.tld/doc?id=1",
"score": 1234,
"originalUrl": "https://shortened.url"
}, },
{ {
"title": "Document 2", "title": "Document 2",
...@@ -206,12 +208,19 @@ TEST_F(DocumentProviderTest, ParseDocumentSearchResults) { ...@@ -206,12 +208,19 @@ TEST_F(DocumentProviderTest, ParseDocumentSearchResults) {
ACMatches matches; ACMatches matches;
provider_->ParseDocumentSearchResults(*response, &matches); provider_->ParseDocumentSearchResults(*response, &matches);
EXPECT_EQ(matches.size(), 2u); EXPECT_EQ(matches.size(), 2u);
EXPECT_EQ(matches[0].contents, base::ASCIIToUTF16("Document 1")); EXPECT_EQ(matches[0].contents, base::ASCIIToUTF16("Document 1"));
EXPECT_EQ(matches[0].destination_url, EXPECT_EQ(matches[0].destination_url,
GURL("https://documentprovider.tld/doc?id=1")); GURL("https://documentprovider.tld/doc?id=1"));
EXPECT_EQ(matches[0].relevance, 1234); // Server-specified.
EXPECT_EQ(matches[0].stripped_destination_url, GURL("https://shortened.url"));
EXPECT_EQ(matches[1].contents, base::ASCIIToUTF16("Document 2")); EXPECT_EQ(matches[1].contents, base::ASCIIToUTF16("Document 2"));
EXPECT_EQ(matches[1].destination_url, EXPECT_EQ(matches[1].destination_url,
GURL("https://documentprovider.tld/doc?id=2")); GURL("https://documentprovider.tld/doc?id=2"));
EXPECT_EQ(matches[1].relevance, 700); // From study default.
EXPECT_TRUE(matches[1].stripped_destination_url.is_empty());
ASSERT_FALSE(provider_->backoff_for_session_); ASSERT_FALSE(provider_->backoff_for_session_);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment