Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(59)

Unified Diff: components/ntp_snippets/ntp_snippets_fetcher.cc

Issue 1922083004: Allow fetching personalized snippets from ChromeReader. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: After code review #2 Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/ntp_snippets/ntp_snippets_fetcher.cc
diff --git a/components/ntp_snippets/ntp_snippets_fetcher.cc b/components/ntp_snippets/ntp_snippets_fetcher.cc
index acad45cf88cc3417be13c39811ca99de7c98a917..0d934b7ecaccb356f4d65dae46e70bad2e0147a3 100644
--- a/components/ntp_snippets/ntp_snippets_fetcher.cc
+++ b/components/ntp_snippets/ntp_snippets_fetcher.cc
@@ -4,6 +4,8 @@
#include "components/ntp_snippets/ntp_snippets_fetcher.h"
+#include <stdlib.h>
+
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/metrics/sparse_histogram.h"
@@ -11,12 +13,20 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
+#include "components/ntp_snippets/ntp_snippets_constants.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
+#include "components/signin/core/browser/signin_manager.h"
+#include "components/signin/core/browser/signin_manager_base.h"
+#include "components/signin/core/browser/signin_tracker.h"
+#include "components/variations/variations_associated_data.h"
#include "google_apis/google_api_keys.h"
#include "net/base/load_flags.h"
#include "net/http/http_request_headers.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_status_code.h"
#include "net/url_request/url_fetcher.h"
+#include "third_party/icu/source/common/unicode/uloc.h"
+#include "third_party/icu/source/common/unicode/utypes.h"
using net::URLFetcher;
using net::URLRequestContextGetter;
@@ -27,19 +37,34 @@ namespace ntp_snippets {
namespace {
+const char kApiScope[] = "https://www.googleapis.com/auth/webhistory";
+
const char kStatusMessageURLRequestErrorFormat[] = "URLRequestStatus error %d";
const char kStatusMessageHTTPErrorFormat[] = "HTTP error %d";
-const char kContentSnippetsServerFormat[] =
+const char kSnippetsServerAuthorized[] =
+ "https://chromereader-pa.googleapis.com/v1/fetch";
+const char kSnippetsServerNonAuthorizedFormat[] =
"https://chromereader-pa.googleapis.com/v1/fetch?key=%s";
Marc Treib 2016/05/04 15:24:24 I meant: Make this "%s?key=%s". Then you won't hav
jkrcal 2016/05/09 11:58:05 Done.
+const char kAuthorizationRequestHeaderFormat[] = "Bearer %s";
+
+// Variation parameter for the variant of fetching to use.
+const char kVariantName[] = "fetching_variant";
+
+// Constants listing possible values of the "variant" parameter.
Marc Treib 2016/05/04 15:24:24 fetching_variant
jkrcal 2016/05/09 11:58:05 Done.
+const char kVariantRestrictedString[] = "restricted";
+const char kVariantPersonalizedString[] = "personalized";
+const char kVariantRestrictedPersonalizedString[] = "restricted_personalized";
const char kRequestParameterFormat[] =
"{"
" \"response_detail_level\": \"STANDARD\","
+ "%s"
Marc Treib 2016/05/04 15:24:24 Can you add a comment like // Gaia ID will be inse
jkrcal 2016/05/09 11:58:06 Done.
" \"advanced_options\": {"
" \"local_scoring_params\": {"
" \"content_params\": {"
" \"only_return_personalized_results\": false"
+ "%s"
" },"
" \"content_restricts\": {"
" \"type\": \"METADATA\","
@@ -56,11 +81,14 @@ const char kRequestParameterFormat[] =
"%s"
" },"
" \"global_scoring_params\": {"
- " \"num_to_return\": %i"
+ " \"num_to_return\": %i,"
+ " \"sort_type\": 1"
" }"
" }"
"}";
+const char kAuthorizationFormat[] = " \"obfuscated_gaia_id\": \"%s\",";
+const char kUserSegmentFormat[] = " \"user_segment\": \"%s\"";
const char kHostRestrictFormat[] =
" ,\"content_selectors\": {"
" \"type\": \"HOST_RESTRICT\","
@@ -70,12 +98,31 @@ const char kHostRestrictFormat[] =
} // namespace
NTPSnippetsFetcher::NTPSnippetsFetcher(
+ SigninManagerBase* signin_manager,
+ OAuth2TokenService* token_service,
scoped_refptr<URLRequestContextGetter> url_request_context_getter,
bool is_stable_channel)
- : url_request_context_getter_(url_request_context_getter),
- is_stable_channel_(is_stable_channel) {}
+ : OAuth2TokenService::Consumer("NTP_snippets"),
+ url_request_context_getter_(url_request_context_getter),
+ signin_manager_(signin_manager),
+ token_service_(token_service),
+ waiting_for_refresh_token_(false),
+ is_stable_channel_(is_stable_channel) {
+ // Parse the variation parameters and set the defaults if missing.
+ std::string variant = variations::GetVariationParamValue(
+ ntp_snippets::kStudyName, kVariantName);
+ if (variant == kVariantRestrictedString)
+ variant_ = kRestricted;
+ else if (variant == kVariantPersonalizedString)
+ variant_ = kPersonalized;
+ else
+ variant_ = kRestrictedPersonalized;
Marc Treib 2016/05/04 15:24:24 Maybe log an error if we're getting a variant stri
jkrcal 2016/05/09 11:58:05 Done.
+}
-NTPSnippetsFetcher::~NTPSnippetsFetcher() {}
+NTPSnippetsFetcher::~NTPSnippetsFetcher() {
+ if (waiting_for_refresh_token_)
+ token_service_->RemoveObserver(this);
+}
std::unique_ptr<NTPSnippetsFetcher::SnippetsAvailableCallbackList::Subscription>
NTPSnippetsFetcher::AddCallback(const SnippetsAvailableCallback& callback) {
@@ -83,27 +130,55 @@ NTPSnippetsFetcher::AddCallback(const SnippetsAvailableCallback& callback) {
}
void NTPSnippetsFetcher::FetchSnippets(const std::set<std::string>& hosts,
+ const std::string& language_code,
int count) {
- const std::string& key = is_stable_channel_
- ? google_apis::GetAPIKey()
- : google_apis::GetNonStableAPIKey();
- std::string url =
- base::StringPrintf(kContentSnippetsServerFormat, key.c_str());
- url_fetcher_ = URLFetcher::Create(GURL(url), URLFetcher::POST, this);
+ hosts_ = hosts;
+
+ // Translate the BCP 47 |language_code| into a posix locale string
+ char locale[ULOC_FULLNAME_CAPACITY];
+ UErrorCode error;
+ uloc_forLanguageTag(language_code.c_str(), locale, ULOC_FULLNAME_CAPACITY,
+ nullptr, &error);
+ DLOG_IF(WARNING, U_ZERO_ERROR != error) <<
+ "Error in translating language code to a locale string: " << error;
+ locale_ = locale;
+
+ count_ = count;
+
+ if (UseAuthentication()) {
+ if (signin_manager_->IsAuthenticated()) {
+ // Signed-in: get OAuth token --> fetch snippets.
+ StartTokenRequest();
+ } else if (signin_manager_->AuthInProgress()) {
+ // Currently signing in: wait for auth to finish (the refresh token) -->
+ // get OAuth token --> fetch snippets.
+ if (!waiting_for_refresh_token_) {
+ // Wait until we get a refresh token.
+ waiting_for_refresh_token_ = true;
+ token_service_->AddObserver(this);
+ }
+ }
+ } else {
+ // Not signed in: fetch snippets (without authentication).
+ FetchSnippetsNonAuthenticated();
+ }
+}
+
+void NTPSnippetsFetcher::FetchSnippetsImpl(const GURL& url,
+ const std::string& auth_header,
+ const std::string& request) {
+ url_fetcher_ = URLFetcher::Create(url, URLFetcher::POST, this);
+
url_fetcher_->SetRequestContext(url_request_context_getter_.get());
url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
net::LOAD_DO_NOT_SAVE_COOKIES);
+
HttpRequestHeaders headers;
+ if (!auth_header.empty())
+ headers.SetHeader("Authorization", auth_header);
headers.SetHeader("Content-Type", "application/json; charset=UTF-8");
url_fetcher_->SetExtraRequestHeaders(headers.ToString());
- std::string host_restricts;
- for (const std::string& host : hosts)
- host_restricts += base::StringPrintf(kHostRestrictFormat, host.c_str());
- url_fetcher_->SetUploadData("application/json",
- base::StringPrintf(kRequestParameterFormat,
- host_restricts.c_str(),
- count));
-
+ url_fetcher_->SetUploadData("application/json", request);
// Fetchers are sometimes cancelled because a network change was detected.
url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(3);
// Try to make fetching the files bit more robust even with poor connection.
@@ -111,6 +186,89 @@ void NTPSnippetsFetcher::FetchSnippets(const std::set<std::string>& hosts,
url_fetcher_->Start();
}
+std::string NTPSnippetsFetcher::GetHostsRestricts() const {
+ std::string host_restricts;
+ if (variant_ == kRestricted || variant_ == kRestrictedPersonalized) {
+ for (const std::string& host : hosts_)
+ host_restricts += base::StringPrintf(kHostRestrictFormat, host.c_str());
+ }
+ return host_restricts;
+}
+
+bool NTPSnippetsFetcher::UseAuthentication() {
+ return (variant_ == kPersonalized ||
+ (variant_ == kRestrictedPersonalized && !hosts_.empty())) &&
+ (signin_manager_->IsAuthenticated() || signin_manager_->AuthInProgress());
+}
+
+void NTPSnippetsFetcher::FetchSnippetsNonAuthenticated() {
+ // When not providing OAuth token, we need to pass the Google API key
+ const std::string& key = is_stable_channel_
+ ? google_apis::GetAPIKey()
+ : google_apis::GetNonStableAPIKey();
+ GURL url(base::StringPrintf(kSnippetsServerNonAuthorizedFormat, key.c_str()));
+
+ FetchSnippetsImpl(url, std::string(),
+ base::StringPrintf(kRequestParameterFormat, "", "",
Marc Treib 2016/05/04 15:24:24 std::string() instead of ""
jkrcal 2016/05/09 11:58:05 Done.
+ GetHostsRestricts().c_str(),
+ count_));
+}
+
+void NTPSnippetsFetcher::FetchSnippetsAuthenticated(
+ const std::string& account_id,
+ const std::string& oauth_access_token) {
+ std::string auth =
+ base::StringPrintf(kAuthorizationFormat, account_id.c_str());
+ std::string user_segment =
+ base::StringPrintf(kUserSegmentFormat, locale_.c_str());
+
+ FetchSnippetsImpl(GURL(kSnippetsServerAuthorized),
+ base::StringPrintf(kAuthorizationRequestHeaderFormat,
+ oauth_access_token.c_str()),
+ base::StringPrintf(kRequestParameterFormat,
+ auth.c_str(),
+ user_segment.c_str(),
+ GetHostsRestricts().c_str(),
+ count_));
+}
+
+void NTPSnippetsFetcher::StartTokenRequest() {
+ OAuth2TokenService::ScopeSet scopes;
+ scopes.insert(kApiScope);
+ oauth_request_ = token_service_->StartRequest(
+ signin_manager_->GetAuthenticatedAccountId(), scopes, this);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// OAuth2TokenService::Consumer overrides
+void NTPSnippetsFetcher::OnGetTokenSuccess(
+ const OAuth2TokenService::Request* request,
+ const std::string& access_token,
+ const base::Time& expiration_time) {
+ oauth_request_.reset();
+
+ FetchSnippetsAuthenticated(request->GetAccountId(), access_token);
+}
+
+void NTPSnippetsFetcher::OnGetTokenFailure(
+ const OAuth2TokenService::Request* request,
+ const GoogleServiceAuthError& error) {
+ oauth_request_.reset();
+ DLOG(ERROR) << "Unable to get token: " << error.ToString()
+ << " - fetching the snippets without authentication.";
+
+ FetchSnippetsNonAuthenticated();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// OAuth2TokenService::Observer overrides
+void NTPSnippetsFetcher::OnRefreshTokenAvailable(
+ const std::string& account_id) {
+ token_service_->RemoveObserver(this);
+ waiting_for_refresh_token_ = false;
+ StartTokenRequest();
+}
+
////////////////////////////////////////////////////////////////////////////////
// URLFetcherDelegate overrides
void NTPSnippetsFetcher::OnURLFetchComplete(const URLFetcher* source) {
@@ -127,14 +285,22 @@ void NTPSnippetsFetcher::OnURLFetchComplete(const URLFetcher* source) {
message = base::StringPrintf(kStatusMessageURLRequestErrorFormat,
status.error());
} else if (source->GetResponseCode() != net::HTTP_OK) {
+ // TODO(jkrcal): https://crbug.com/609084
+ // We need to deal with the edge case again where the auth
+ // token expires just before we send the request (in which case we need to
+ // fetch a new auth token). We should extract that into a common class
+ // instead of adding it to every single class that uses auth tokens.
message = base::StringPrintf(kStatusMessageHTTPErrorFormat,
source->GetResponseCode());
}
std::string response;
if (!message.empty()) {
+ std::string error_response;
+ source->GetResponseAsString(&error_response);
DLOG(WARNING) << message << " while trying to download "
- << source->GetURL().spec();
+ << source->GetURL().spec() << ": "
+ << error_response;
} else {
bool stores_result_to_string = source->GetResponseAsString(&response);
« no previous file with comments | « components/ntp_snippets/ntp_snippets_fetcher.h ('k') | components/ntp_snippets/ntp_snippets_fetcher_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698