Skip to content

Commit

Permalink
[EOC] Implement fetching of suggestions
Browse files Browse the repository at this point in the history
Add logic for building request protos and parsing response protos
Add and record fetch-related histograms
Add networking logic
Update the traffic annotation and un-deprecate it
Add tests for the above

Bug: 807697
Change-Id: I4d3196190a9e0340bc8d4022af166596209cdc74
Reviewed-on: https://chromium-review.googlesource.com/999036
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Adam Michalik <xyzzyz@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550268}
  • Loading branch information
Patrick Noland authored and Commit Bot committed Apr 12, 2018
1 parent a28eea0 commit 393e49b
Show file tree
Hide file tree
Showing 12 changed files with 726 additions and 32 deletions.
2 changes: 2 additions & 0 deletions components/ntp_snippets/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ static_library("ntp_snippets") {
"//services/network/public/cpp",
"//services/network/public/mojom",
"//third_party/icu/",
"//third_party/protobuf:protobuf_lite",
"//ui/gfx",
]
}
Expand Down Expand Up @@ -245,6 +246,7 @@ source_set("unit_tests") {
}

deps = [
":explore_protos",
":ntp_snippets",
":test_support",
"//base",
Expand Down
1 change: 1 addition & 0 deletions components/ntp_snippets/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ include_rules = [
"+services/network/public/cpp",
"+services/network/public/mojom",
"+services/network/test",
"+third_party/protobuf",
"+ui/base",
"+ui/gfx",
]
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ ContextualContentSuggestionsService::ContextualContentSuggestionsService(
ContextualContentSuggestionsService::~ContextualContentSuggestionsService() =
default;

// TODO(pnoland): remove this
void ContextualContentSuggestionsService::FetchContextualSuggestions(
const GURL& url,
FetchContextualSuggestionsCallback callback) {}

void ContextualContentSuggestionsService::FetchContextualSuggestionClusters(
const GURL& url,
FetchClustersCallback callback) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,6 @@ class ContextualContentSuggestionsService : public KeyedService {
metrics_reporter_provider);
~ContextualContentSuggestionsService() override;

using FetchContextualSuggestionsCallback =
base::OnceCallback<void(Status status_code,
const GURL& url,
std::vector<ContentSuggestion> suggestions)>;

// Asynchronously fetches contextual suggestions for the given URL.
virtual void FetchContextualSuggestions(
const GURL& url,
FetchContextualSuggestionsCallback callback);

// Asynchronously fetches contextual suggestions for the given URL.
virtual void FetchContextualSuggestionClusters(
const GURL& url,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_unittest_util.h"

using contextual_suggestions::ClusterBuilder;
using testing::_;
using testing::AllOf;
using testing::ElementsAre;
Expand All @@ -41,12 +42,36 @@ namespace ntp_snippets {

namespace {

// TODO(pnoland): Re-implement for new fetcher.
class MockClustersCallback {
public:
void Done(std::string peek_text, std::vector<Cluster> clusters) {
EXPECT_FALSE(has_run);
has_run = true;
response_peek_text = peek_text;
response_clusters = std::move(clusters);
}

bool has_run = false;
std::string response_peek_text;
std::vector<Cluster> response_clusters;
};

// Always fetches the result that was set by SetFakeResponse.
class FakeContextualSuggestionsFetcher : public ContextualSuggestionsFetcher {
public:
void FetchContextualSuggestionsClusters(
const GURL& url,
FetchClustersCallback callback) override {}
FetchClustersCallback callback) override {
std::move(callback).Run("peek text", std::move(fake_suggestions_));
fake_suggestions_.clear();
}

void SetFakeResponse(std::vector<Cluster> fake_suggestions) {
fake_suggestions_ = std::move(fake_suggestions);
}

private:
std::vector<Cluster> fake_suggestions_;
};

// Always fetches a fake image if the given URL is valid.
Expand Down Expand Up @@ -115,4 +140,28 @@ TEST_F(ContextualContentSuggestionsServiceTest,
base::RunLoop().RunUntilIdle();
}

TEST_F(ContextualContentSuggestionsServiceTest,
ShouldFetchContextualSuggestionsClusters) {
MockClustersCallback mock_callback;
std::vector<Cluster> clusters;
GURL context_url("http://www.from.url");

clusters.emplace_back(ClusterBuilder("Title")
.AddSuggestion(SuggestionBuilder(context_url)
.Title("Title1")
.PublisherName("from.url")
.Snippet("Summary")
.ImageId("abc")
.Build())
.Build());

fetcher()->SetFakeResponse(std::move(clusters));
source()->FetchContextualSuggestionClusters(
context_url, base::BindOnce(&MockClustersCallback::Done,
base::Unretained(&mock_callback)));
base::RunLoop().RunUntilIdle();

EXPECT_TRUE(mock_callback.has_run);
}

} // namespace ntp_snippets
195 changes: 191 additions & 4 deletions components/ntp_snippets/contextual/contextual_suggestions_fetch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,105 @@
#include "components/ntp_snippets/contextual/proto/get_pivots_response.pb.h"
#include "components/variations/net/variations_http_headers.h"
#include "net/base/load_flags.h"
#include "net/http/http_status_code.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/cpp/simple_url_loader.h"
#include "third_party/protobuf/src/google/protobuf/io/coded_stream.h"

using base::Base64Encode;

namespace contextual_suggestions {

namespace {

static constexpr char kFetchEndpoint[] =
"https://www.google.com/httpservice/web/ExploreService/GetPivots/";

static constexpr int kNumberOfSuggestionsToFetch = 10;
static constexpr int kMinNumberOfClusters = 1;
static constexpr int kMaxNumberOfClusters = 5;

ContextualSuggestion ItemToSuggestion(const PivotItem& item) {
PivotDocument document = item.document();

return SuggestionBuilder(GURL(document.url().raw_url()))
.Title(document.title())
.Snippet(document.summary())
.PublisherName(document.site_name())
.ImageId(document.image().id().encrypted_docid())
.FaviconImageId(document.favicon_image().id().encrypted_docid())
.Build();
}

Cluster PivotToCluster(const PivotCluster& pivot) {
ClusterBuilder cluster_builder(pivot.label().label());
for (PivotItem item : pivot.item()) {
cluster_builder.AddSuggestion(ItemToSuggestion(item));
}

return cluster_builder.Build();
}

std::vector<Cluster> ClustersFromResponse(
const GetPivotsResponse& response_proto) {
std::vector<Cluster> clusters;
Pivots pivots = response_proto.pivots();

if (pivots.item_size() > 0) {
// If the first item is a cluster, we can assume they all are.
if (pivots.item()[0].has_cluster()) {
clusters.reserve(response_proto.pivots().item_size());
for (auto item : response_proto.pivots().item()) {
if (item.has_cluster()) {
PivotCluster pivot_cluster = item.cluster();
clusters.emplace_back(PivotToCluster(pivot_cluster));
}
}
} else if (pivots.item()[0].has_document()) {
Cluster single_cluster;
for (auto item : pivots.item()) {
single_cluster.suggestions.emplace_back(ItemToSuggestion(item));
}
clusters.emplace_back(std::move(single_cluster));
}
}

return clusters;
}

std::string PeekTextFromResponse(const GetPivotsResponse& response_proto) {
return response_proto.pivots().peek_text().text();
}

const std::string SerializedPivotsRequest(const std::string& url,
const std::string& bcp_language) {
GetPivotsRequest pivot_request;

SearchApiRequestContext* search_context = pivot_request.mutable_context();
search_context->mutable_localization_context()->set_spoken_language(
bcp_language);

GetPivotsQuery* query = pivot_request.mutable_query();
query->add_context()->set_url(url);

PivotDocumentParams* params = query->mutable_document_params();
params->set_enabled(true);
params->set_num(kNumberOfSuggestionsToFetch);
params->set_enable_images(true);
params->set_image_aspect_ratio(PivotDocumentParams::SQUARE);

PivotClusteringParams* cluster_params = query->mutable_clustering_params();
cluster_params->set_enabled(true);
cluster_params->set_min(kMinNumberOfClusters);
cluster_params->set_max(kMaxNumberOfClusters);

query->mutable_peek_text_params()->set_enabled(true);

return pivot_request.SerializeAsString();
}

} // namespace

ContextualSuggestionsFetch::ContextualSuggestionsFetch(
const GURL& url,
const std::string& bcp_language_code)
Expand All @@ -41,18 +129,117 @@ const std::string ContextualSuggestionsFetch::GetFetchEndpoint() {

void ContextualSuggestionsFetch::Start(
FetchClustersCallback callback,
const scoped_refptr<network::SharedURLLoaderFactory>& loader_factory) {}
const scoped_refptr<network::SharedURLLoaderFactory>& loader_factory) {
request_completed_callback_ = std::move(callback);
url_loader_ = MakeURLLoader();
url_loader_->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
loader_factory.get(),
base::BindOnce(&ContextualSuggestionsFetch::OnURLLoaderComplete,
base::Unretained(this)));
}

std::unique_ptr<network::SimpleURLLoader>
ContextualSuggestionsFetch::MakeURLLoader() const {
return nullptr;
// TODO(pnoland, https://crbug.com/831693): Update this once there's an
// opt-out setting.
net::NetworkTrafficAnnotationTag traffic_annotation =
net::DefineNetworkTrafficAnnotation("ntp_contextual_suggestions_fetch",
R"(
semantics {
sender: "Contextual Suggestions Fetch"
description:
"Chromium can show contextual suggestions that are related to the "
"currently visited page."
trigger:
"Triggered when a page is visited and stayed on for more than a "
"few seconds."
data:
"The URL of the current tab and the ui language."
destination: GOOGLE_OWNED_SERVICE
}
policy {
cookies_allowed: NO
setting:
"This feature can be disabled by the flag "
"enable-contextual-suggestions-bottom-sheet."
policy_exception_justification: "Not implemented. The feature is "
"currently Android-only and disabled for all enterprise users. "
"A policy will be added before enabling for enterprise users."
})");

auto resource_request = std::make_unique<network::ResourceRequest>();

resource_request->url = GURL(GetFetchEndpoint());
// TODO(pnoland): include cookies once the suggestions endpoint can make use
// of them.
resource_request->load_flags =
net::LOAD_BYPASS_CACHE | net::LOAD_DO_NOT_SAVE_COOKIES |
net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SEND_AUTH_DATA;
resource_request->headers = MakeHeaders();
resource_request->method = "GET";

auto simple_loader = network::SimpleURLLoader::Create(
std::move(resource_request), traffic_annotation);
simple_loader->SetAllowHttpErrorResults(true);
return simple_loader;
}

net::HttpRequestHeaders ContextualSuggestionsFetch::MakeHeaders() const {
return net::HttpRequestHeaders();
net::HttpRequestHeaders headers;
std::string serialized_request_body =
SerializedPivotsRequest(url_.spec(), bcp_language_code_);
std::string base64_encoded_body;
base::Base64UrlEncode(serialized_request_body,
base::Base64UrlEncodePolicy::INCLUDE_PADDING,
&base64_encoded_body);
headers.SetHeader("X-Protobuffer-Request-Payload", base64_encoded_body);
variations::AppendVariationHeaders(url_, variations::InIncognito::kNo,
variations::SignedIn::kNo, &headers);

UMA_HISTOGRAM_COUNTS_1M(
"ContextualSuggestions.FetchRequestProtoSizeKB",
static_cast<int>(base64_encoded_body.length() / 1024));

return headers;
}

void ContextualSuggestionsFetch::OnURLLoaderComplete(
std::unique_ptr<std::string> result) {}
std::unique_ptr<std::string> result) {
std::vector<Cluster> clusters;
std::string peek_text;

base::UmaHistogramSparse("ContextualSuggestions.FetchErrorCode",
url_loader_->NetError());

if (result) {
int32_t response_code =
url_loader_->ResponseInfo()->headers->response_code();

if (response_code == net::HTTP_OK) {
// The response comes in the format (length, bytes) where length is a
// varint32 encoded int. Rather than hand-rolling logic to skip the
// length(which we don't actually need), we use EncodedInputStream to
// skip past it, then parse the remainder of the stream.
google::protobuf::io::CodedInputStream coded_stream(
reinterpret_cast<const uint8_t*>(result->data()), result->length());
uint32_t response_size;
if (coded_stream.ReadVarint32(&response_size) && response_size != 0) {
GetPivotsResponse response_proto;
if (response_proto.MergePartialFromCodedStream(&coded_stream)) {
clusters = ClustersFromResponse(response_proto);
peek_text = PeekTextFromResponse(response_proto);
}
}
}

UMA_HISTOGRAM_COUNTS_1M("ContextualSuggestions.FetchResponseSizeKB",
static_cast<int>(result->length() / 1024));

base::UmaHistogramSparse("ContextualSuggestions.FetchResponseCode",
response_code);
}

std::move(request_completed_callback_).Run(peek_text, std::move(clusters));
}

} // namespace contextual_suggestions
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,34 @@ ContextualSuggestionsFetcherImpl::ContextualSuggestionsFetcherImpl(
const scoped_refptr<network::SharedURLLoaderFactory>& loader_factory,
const std::string& application_language_code)
: loader_factory_(loader_factory),
bcp_language_code_(application_language_code) {
// Use loader_factory_ to suppress unused variable compiler warning.
loader_factory_.get();
}
bcp_language_code_(application_language_code) {}

ContextualSuggestionsFetcherImpl::~ContextualSuggestionsFetcherImpl() = default;

void ContextualSuggestionsFetcherImpl::FetchContextualSuggestionsClusters(
const GURL& url,
FetchClustersCallback callback) {}
FetchClustersCallback callback) {
auto fetch =
std::make_unique<ContextualSuggestionsFetch>(url, bcp_language_code_);
ContextualSuggestionsFetch* fetch_unowned = fetch.get();
pending_requests_.emplace(std::move(fetch));

FetchClustersCallback internal_callback = base::BindOnce(
&ContextualSuggestionsFetcherImpl::FetchFinished, base::Unretained(this),
fetch_unowned, std::move(callback));
fetch_unowned->Start(std::move(internal_callback), loader_factory_);
}

void ContextualSuggestionsFetcherImpl::FetchFinished(
ContextualSuggestionsFetch* fetch,
FetchClustersCallback callback,
std::string peek_text,
std::vector<Cluster> clusters) {}
std::vector<Cluster> clusters) {
auto fetch_iterator = pending_requests_.find(fetch);
CHECK(fetch_iterator != pending_requests_.end());
pending_requests_.erase(fetch_iterator);

std::move(callback).Run(peek_text, std::move(clusters));
}

} // namespace ntp_snippets
Loading

0 comments on commit 393e49b

Please sign in to comment.