Commit c5a8652b authored by Roger McFarlane's avatar Roger McFarlane Committed by Commit Bot

Plumb session tokens for signed-out feed requests.

Bug: 1110384
Change-Id: I7c0324071625e4bf25e3b797d5a1fb60bdedd1c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476134Reviewed-by: default avatarDan H <harringtond@chromium.org>
Reviewed-by: default avatarCarlos Knippschild <carlosk@chromium.org>
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826859}
parent 49bce2f7
...@@ -359,6 +359,10 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand ...@@ -359,6 +359,10 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
@Override @Override
public String getAccountName() { public String getAccountName() {
// Don't return account name if there's a signed-out session ID.
if (!getSignedOutSessionId().isEmpty()) {
return "";
}
assert ThreadUtils.runningOnUiThread(); assert ThreadUtils.runningOnUiThread();
CoreAccountInfo primaryAccount = CoreAccountInfo primaryAccount =
IdentityServicesProvider.get() IdentityServicesProvider.get()
...@@ -375,9 +379,20 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand ...@@ -375,9 +379,20 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
@Override @Override
public String getClientInstanceId() { public String getClientInstanceId() {
// Don't return client instance id if there's a signed-out session ID.
if (!getSignedOutSessionId().isEmpty()) {
return "";
}
assert ThreadUtils.runningOnUiThread(); assert ThreadUtils.runningOnUiThread();
return FeedServiceBridge.getClientInstanceId(); return FeedServiceBridge.getClientInstanceId();
} }
@Override
public String getSignedOutSessionId() {
assert ThreadUtils.runningOnUiThread();
return FeedStreamSurfaceJni.get().getSessionId(
mNativeFeedStreamSurface, FeedStreamSurface.this);
}
} }
/** /**
...@@ -1173,6 +1188,7 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand ...@@ -1173,6 +1188,7 @@ public class FeedStreamSurface implements SurfaceActionsHandler, FeedActionsHand
long init(FeedStreamSurface caller); long init(FeedStreamSurface caller);
boolean isActivityLoggingEnabled(long nativeFeedStreamSurface, FeedStreamSurface caller); boolean isActivityLoggingEnabled(long nativeFeedStreamSurface, FeedStreamSurface caller);
int[] getExperimentIds(); int[] getExperimentIds();
String getSessionId(long nativeFeedStreamSurface, FeedStreamSurface caller);
void reportFeedViewed(long nativeFeedStreamSurface, FeedStreamSurface caller); void reportFeedViewed(long nativeFeedStreamSurface, FeedStreamSurface caller);
void reportSliceViewed( void reportSliceViewed(
long nativeFeedStreamSurface, FeedStreamSurface caller, String sliceId); long nativeFeedStreamSurface, FeedStreamSurface caller, String sliceId);
......
...@@ -172,6 +172,13 @@ bool FeedStreamSurface::IsActivityLoggingEnabled( ...@@ -172,6 +172,13 @@ bool FeedStreamSurface::IsActivityLoggingEnabled(
return feed_stream_api_ && feed_stream_api_->IsActivityLoggingEnabled(); return feed_stream_api_ && feed_stream_api_->IsActivityLoggingEnabled();
} }
base::android::ScopedJavaLocalRef<jstring> FeedStreamSurface::GetSessionId(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
return base::android::ConvertUTF8ToJavaString(
env, feed_stream_api_ ? feed_stream_api_->GetSessionId() : std::string());
}
void FeedStreamSurface::ReportOpenAction( void FeedStreamSurface::ReportOpenAction(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jobject>& obj, const JavaParamRef<jobject>& obj,
......
...@@ -66,11 +66,16 @@ class FeedStreamSurface : public FeedStreamApi::SurfaceInterface { ...@@ -66,11 +66,16 @@ class FeedStreamSurface : public FeedStreamApi::SurfaceInterface {
void SurfaceClosed(JNIEnv* env, void SurfaceClosed(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj); const base::android::JavaParamRef<jobject>& obj);
// Is activity Loggine enabled (ephemeral). // Is activity logging enabled (ephemeral).
bool IsActivityLoggingEnabled( bool IsActivityLoggingEnabled(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj); const base::android::JavaParamRef<jobject>& obj);
// Get the signed-out session id, if any (ephemeral).
base::android::ScopedJavaLocalRef<jstring> GetSessionId(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
// Event reporting functions. These have no side-effect beyond recording // Event reporting functions. These have no side-effect beyond recording
// metrics. See |FeedStreamApi| for definitions. // metrics. See |FeedStreamApi| for definitions.
void ReportSliceViewed(JNIEnv* env, void ReportSliceViewed(JNIEnv* env,
......
...@@ -28,6 +28,7 @@ proto_library("proto_v2") { ...@@ -28,6 +28,7 @@ proto_library("proto_v2") {
"v2/ui.proto", "v2/ui.proto",
"v2/wire/action_payload.proto", "v2/wire/action_payload.proto",
"v2/wire/capability.proto", "v2/wire/capability.proto",
"v2/wire/chrome_client_info.proto",
"v2/wire/chrome_feed_response_metadata.proto", "v2/wire/chrome_feed_response_metadata.proto",
"v2/wire/chrome_fulfillment_info.proto", "v2/wire/chrome_fulfillment_info.proto",
"v2/wire/client_info.proto", "v2/wire/client_info.proto",
......
...@@ -51,15 +51,24 @@ message StreamData { ...@@ -51,15 +51,24 @@ message StreamData {
bool logging_enabled = 8; bool logging_enabled = 8;
// Has the privacy notice been fulfilled? // Has the privacy notice been fulfilled?
bool privacy_notice_fulfilled = 9; bool privacy_notice_fulfilled = 9;
reserved 3, 5; reserved 3, 5;
} }
// Data that doesn't belong to a stream. // Data that doesn't belong to a stream.
message Metadata { message Metadata {
// Session identifier used for signed-out feed requests and interactions.
message SessionID {
string token = 1;
int64 expiry_time_ms = 2;
}
// Token used to read or write to the same storage. // Token used to read or write to the same storage.
bytes consistency_token = 1; bytes consistency_token = 1;
// ID for the next pending action. // ID for the next pending action.
int32 next_action_id = 2; int32 next_action_id = 2;
// The most recent session identifier.
SessionID session_id = 3;
} }
// A set of StreamStructures that should be applied to a stream. // A set of StreamStructures that should be applied to a stream.
......
// Copyright 2020 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.
syntax = "proto2";
package feedwire;
option optimize_for = LITE_RUNTIME;
// Information about the client performing the request, relevant only to Chrome
// surfaces.
message ChromeClientInfo {
// The signed-out session identifier (Zwieback) token, for clients which embed
// this information in-band instead of using an HTTP Cookie.
optional string session_id = 3;
}
...@@ -11,4 +11,5 @@ option optimize_for = LITE_RUNTIME; ...@@ -11,4 +11,5 @@ option optimize_for = LITE_RUNTIME;
message ChromeFeedResponseMetadata { message ChromeFeedResponseMetadata {
optional bool privacy_notice_fulfilled = 1; optional bool privacy_notice_fulfilled = 1;
optional bool logging_enabled = 2; optional bool logging_enabled = 2;
optional string session_id = 3;
} }
...@@ -8,6 +8,7 @@ package feedwire; ...@@ -8,6 +8,7 @@ package feedwire;
option optimize_for = LITE_RUNTIME; option optimize_for = LITE_RUNTIME;
import "components/feed/core/proto/v2/wire/chrome_client_info.proto";
import "components/feed/core/proto/v2/wire/display_info.proto"; import "components/feed/core/proto/v2/wire/display_info.proto";
import "components/feed/core/proto/v2/wire/version.proto"; import "components/feed/core/proto/v2/wire/version.proto";
...@@ -58,4 +59,8 @@ message ClientInfo { ...@@ -58,4 +59,8 @@ message ClientInfo {
// this comes from GServices check-in which uses the SIM card MCC (mobile // this comes from GServices check-in which uses the SIM card MCC (mobile
// country code), with fallback to IP geo lookup. // country code), with fallback to IP geo lookup.
optional string device_country = 9; optional string device_country = 9;
// Information about the client performing the request, relevant only to
// Chrome surfaces.
optional ChromeClientInfo chrome_client_info = 338478298;
} }
...@@ -76,6 +76,16 @@ void OverrideWithFinch(Config* config) { ...@@ -76,6 +76,16 @@ void OverrideWithFinch(Config* config) {
kInterestFeedV2, "upload_actions_on_enter_background", kInterestFeedV2, "upload_actions_on_enter_background",
config->upload_actions_on_enter_background); config->upload_actions_on_enter_background);
config->send_signed_out_session_logs =
base::GetFieldTrialParamByFeatureAsBool(
kInterestFeedV2, "send_signed_out_session_logs",
config->send_signed_out_session_logs);
config->session_id_max_age =
base::TimeDelta::FromDays(base::GetFieldTrialParamByFeatureAsInt(
kInterestFeedV2, "session_id_max_age_days",
config->session_id_max_age.InDays()));
// Erase any capabilities with "enable_CAPABILITY = false" set. // Erase any capabilities with "enable_CAPABILITY = false" set.
base::EraseIf(config->experimental_capabilities, CapabilityDisabled); base::EraseIf(config->experimental_capabilities, CapabilityDisabled);
} }
......
...@@ -38,6 +38,10 @@ struct Config { ...@@ -38,6 +38,10 @@ struct Config {
int load_more_trigger_lookahead = 5; int load_more_trigger_lookahead = 5;
// Whether to attempt uploading actions when Chrome is hidden. // Whether to attempt uploading actions when Chrome is hidden.
bool upload_actions_on_enter_background = true; bool upload_actions_on_enter_background = true;
// Whether to send (pseudonymous) logs for signed-out sessions.
bool send_signed_out_session_logs = true;
// The max age of a signed-out session token.
base::TimeDelta session_id_max_age = base::TimeDelta::FromDays(30);
// Set of optional capabilities included in requests. See // Set of optional capabilities included in requests. See
// CreateFeedQueryRequest() for required capabilities. // CreateFeedQueryRequest() for required capabilities.
base::flat_set<feedwire::Capability> experimental_capabilities = { base::flat_set<feedwire::Capability> experimental_capabilities = {
......
...@@ -113,7 +113,7 @@ void FeedStream::Metadata::Populate(feedstore::Metadata metadata) { ...@@ -113,7 +113,7 @@ void FeedStream::Metadata::Populate(feedstore::Metadata metadata) {
metadata_ = std::move(metadata); metadata_ = std::move(metadata);
} }
std::string FeedStream::Metadata::GetConsistencyToken() const { const std::string& FeedStream::Metadata::GetConsistencyToken() const {
return metadata_.consistency_token(); return metadata_.consistency_token();
} }
...@@ -122,6 +122,36 @@ void FeedStream::Metadata::SetConsistencyToken(std::string consistency_token) { ...@@ -122,6 +122,36 @@ void FeedStream::Metadata::SetConsistencyToken(std::string consistency_token) {
store_->WriteMetadata(metadata_, base::DoNothing()); store_->WriteMetadata(metadata_, base::DoNothing());
} }
const std::string& FeedStream::Metadata::GetSessionIdToken() const {
return metadata_.session_id().token();
}
base::Time FeedStream::Metadata::GetSessionIdExpiryTime() const {
return base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromMilliseconds(
metadata_.session_id().expiry_time_ms()));
}
void FeedStream::Metadata::SetSessionId(std::string token,
base::Time expiry_time) {
feedstore::Metadata::SessionID* session_id = metadata_.mutable_session_id();
session_id->set_token(std::move(token));
session_id->set_expiry_time_ms(
expiry_time.ToDeltaSinceWindowsEpoch().InMilliseconds());
store_->WriteMetadata(metadata_, base::DoNothing());
}
void FeedStream::Metadata::MaybeUpdateSessionId(
base::Optional<std::string> token,
const base::Clock* clock) {
if (token && metadata_.session_id().token() != *token) {
base::Time expiry_time =
token->empty() ? base::Time()
: clock->Now() + GetFeedConfig().session_id_max_age;
SetSessionId(*token, expiry_time);
}
}
LocalActionId FeedStream::Metadata::GetNextActionId() { LocalActionId FeedStream::Metadata::GetNextActionId() {
uint32_t id = metadata_.next_action_id(); uint32_t id = metadata_.next_action_id();
// Never use 0, as that's an invalid LocalActionId. // Never use 0, as that's an invalid LocalActionId.
...@@ -239,7 +269,13 @@ bool FeedStream::IsActivityLoggingEnabled() const { ...@@ -239,7 +269,13 @@ bool FeedStream::IsActivityLoggingEnabled() const {
void FeedStream::UpdateIsActivityLoggingEnabled() { void FeedStream::UpdateIsActivityLoggingEnabled() {
is_activity_logging_enabled_ = is_activity_logging_enabled_ =
model_ && model_->signed_in() && model_->logging_enabled(); model_ &&
((model_->signed_in() && model_->logging_enabled()) ||
(!model_->signed_in() && GetFeedConfig().send_signed_out_session_logs));
}
std::string FeedStream::GetSessionId() const {
return GetMetadata()->GetSessionIdToken();
} }
void FeedStream::AttachSurface(SurfaceInterface* surface) { void FeedStream::AttachSurface(SurfaceInterface* surface) {
...@@ -302,7 +338,7 @@ bool FeedStream::IsArticlesListVisible() { ...@@ -302,7 +338,7 @@ bool FeedStream::IsArticlesListVisible() {
return profile_prefs_->GetBoolean(prefs::kArticlesListVisible); return profile_prefs_->GetBoolean(prefs::kArticlesListVisible);
} }
std::string FeedStream::GetClientInstanceId() { std::string FeedStream::GetClientInstanceId() const {
return prefs::GetClientInstanceId(*profile_prefs_); return prefs::GetClientInstanceId(*profile_prefs_);
} }
...@@ -571,14 +607,38 @@ bool FeedStream::ShouldForceSignedOutFeedQueryRequest() const { ...@@ -571,14 +607,38 @@ bool FeedStream::ShouldForceSignedOutFeedQueryRequest() const {
return base::TimeTicks::Now() < signed_out_refreshes_until_; return base::TimeTicks::Now() < signed_out_refreshes_until_;
} }
RequestMetadata FeedStream::GetRequestMetadata() { RequestMetadata FeedStream::GetRequestMetadata(bool is_for_next_page) const {
RequestMetadata result; RequestMetadata result;
result.chrome_info = chrome_info_; result.chrome_info = chrome_info_;
result.display_metrics = delegate_->GetDisplayMetrics(); result.display_metrics = delegate_->GetDisplayMetrics();
result.language_tag = delegate_->GetLanguageTag(); result.language_tag = delegate_->GetLanguageTag();
result.client_instance_id = GetClientInstanceId();
result.notice_card_acknowledged = result.notice_card_acknowledged =
notice_card_tracker_.HasAcknowledgedNoticeCard(); notice_card_tracker_.HasAcknowledgedNoticeCard();
if (is_for_next_page) {
// If we are continuing an existing feed, use whatever session continuity
// mechanism is currently associated with the stream: client-instance-id
// for signed-in feed, session_id token for signed-out.
DCHECK(model_);
if (model_->signed_in()) {
result.client_instance_id = GetClientInstanceId();
} else {
result.session_id = GetMetadata()->GetSessionIdToken();
}
} else {
// The request is for the first page of the feed. Use client_instance_id
// for signed in requests and session_id token (if any, and not expired)
// for signed-out.
if (delegate_->IsSignedIn() && !ShouldForceSignedOutFeedQueryRequest()) {
result.client_instance_id = GetClientInstanceId();
} else if (!GetMetadata()->GetSessionIdToken().empty() &&
GetMetadata()->GetSessionIdExpiryTime() > clock_->Now()) {
result.session_id = GetMetadata()->GetSessionIdToken();
}
}
DCHECK(result.session_id.empty() || result.client_instance_id.empty());
return result; return result;
} }
...@@ -739,6 +799,7 @@ void FeedStream::ReportSliceViewed(SurfaceId surface_id, ...@@ -739,6 +799,7 @@ void FeedStream::ReportSliceViewed(SurfaceId surface_id,
metrics_reporter_->ContentSliceViewed(surface_id, index); metrics_reporter_->ContentSliceViewed(surface_id, index);
} }
} }
// TODO(crbug/1147237): Rename this method and related members?
bool FeedStream::CanUploadActions() const { bool FeedStream::CanUploadActions() const {
return can_upload_actions_with_notice_card_ || return can_upload_actions_with_notice_card_ ||
!prefs::GetLastFetchHadNoticeCard(*profile_prefs_); !prefs::GetLastFetchHadNoticeCard(*profile_prefs_);
......
...@@ -92,9 +92,15 @@ class FeedStream : public FeedStreamApi, ...@@ -92,9 +92,15 @@ class FeedStream : public FeedStreamApi,
void Populate(feedstore::Metadata metadata); void Populate(feedstore::Metadata metadata);
std::string GetConsistencyToken() const; const std::string& GetConsistencyToken() const;
void SetConsistencyToken(std::string consistency_token); void SetConsistencyToken(std::string consistency_token);
const std::string& GetSessionIdToken() const;
base::Time GetSessionIdExpiryTime() const;
void SetSessionId(std::string token, base::Time expiry_time);
void MaybeUpdateSessionId(base::Optional<std::string> token,
const base::Clock* clock);
LocalActionId GetNextActionId(); LocalActionId GetNextActionId();
private: private:
...@@ -125,11 +131,12 @@ class FeedStream : public FeedStreamApi, ...@@ -125,11 +131,12 @@ class FeedStream : public FeedStreamApi,
// FeedStreamApi. // FeedStreamApi.
bool IsActivityLoggingEnabled() const override; bool IsActivityLoggingEnabled() const override;
std::string GetSessionId() const override;
void AttachSurface(SurfaceInterface*) override; void AttachSurface(SurfaceInterface*) override;
void DetachSurface(SurfaceInterface*) override; void DetachSurface(SurfaceInterface*) override;
void SetArticlesListVisible(bool is_visible) override; void SetArticlesListVisible(bool is_visible) override;
bool IsArticlesListVisible() override; bool IsArticlesListVisible() override;
std::string GetClientInstanceId() override; std::string GetClientInstanceId() const override;
void ExecuteRefreshTask() override; void ExecuteRefreshTask() override;
ImageFetchId FetchImage( ImageFetchId FetchImage(
const GURL& url, const GURL& url,
...@@ -215,6 +222,7 @@ class FeedStream : public FeedStreamApi, ...@@ -215,6 +222,7 @@ class FeedStream : public FeedStreamApi,
FeedStore* GetStore() { return store_; } FeedStore* GetStore() { return store_; }
RequestThrottler* GetRequestThrottler() { return &request_throttler_; } RequestThrottler* GetRequestThrottler() { return &request_throttler_; }
Metadata* GetMetadata() { return &metadata_; } Metadata* GetMetadata() { return &metadata_; }
const Metadata* GetMetadata() const { return &metadata_; }
MetricsReporter* GetMetricsReporter() const { return metrics_reporter_; } MetricsReporter* GetMetricsReporter() const { return metrics_reporter_; }
// Returns the time of the last content fetch. // Returns the time of the last content fetch.
...@@ -256,7 +264,7 @@ class FeedStream : public FeedStreamApi, ...@@ -256,7 +264,7 @@ class FeedStream : public FeedStreamApi,
const base::Clock* GetClock() const { return clock_; } const base::Clock* GetClock() const { return clock_; }
const base::TickClock* GetTickClock() const { return tick_clock_; } const base::TickClock* GetTickClock() const { return tick_clock_; }
RequestMetadata GetRequestMetadata(); RequestMetadata GetRequestMetadata(bool is_for_next_page) const;
const WireResponseTranslator* GetWireResponseTranslator() const { const WireResponseTranslator* GetWireResponseTranslator() const {
return wire_response_translator_; return wire_response_translator_;
......
...@@ -36,7 +36,7 @@ void NoticeCardTracker::OnOpenAction(int index) { ...@@ -36,7 +36,7 @@ void NoticeCardTracker::OnOpenAction(int index) {
MaybeUpdateNoticeCardClicksCount(index); MaybeUpdateNoticeCardClicksCount(index);
} }
bool NoticeCardTracker::HasAcknowledgedNoticeCard() { bool NoticeCardTracker::HasAcknowledgedNoticeCard() const {
constexpr char kNoticeCardViewsCountThresholdParamName[] = constexpr char kNoticeCardViewsCountThresholdParamName[] =
"notice-card-views-count-threshold"; "notice-card-views-count-threshold";
constexpr char kNoticeCardClicksCountThresholdParamName[] = constexpr char kNoticeCardClicksCountThresholdParamName[] =
......
...@@ -27,7 +27,7 @@ class NoticeCardTracker { ...@@ -27,7 +27,7 @@ class NoticeCardTracker {
// Indicates whether there were enough views or clicks done on the notice // Indicates whether there were enough views or clicks done on the notice
// card to consider it as acknowledged by the user. // card to consider it as acknowledged by the user.
bool HasAcknowledgedNoticeCard(); bool HasAcknowledgedNoticeCard() const;
private: private:
bool HasNoticeCardActionsCountPrerequisites(int index); bool HasNoticeCardActionsCountPrerequisites(int index);
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "components/feed/core/proto/v2/store.pb.h" #include "components/feed/core/proto/v2/store.pb.h"
#include "components/feed/core/proto/v2/wire/capability.pb.h" #include "components/feed/core/proto/v2/wire/capability.pb.h"
#include "components/feed/core/proto/v2/wire/chrome_client_info.pb.h"
#include "components/feed/core/proto/v2/wire/feed_request.pb.h" #include "components/feed/core/proto/v2/wire/feed_request.pb.h"
#include "components/feed/core/proto/v2/wire/request.pb.h" #include "components/feed/core/proto/v2/wire/request.pb.h"
#include "components/feed/core/v2/config.h" #include "components/feed/core/v2/config.h"
...@@ -125,9 +126,15 @@ feedwire::Request CreateFeedQueryRequest( ...@@ -125,9 +126,15 @@ feedwire::Request CreateFeedQueryRequest(
*feed_request.mutable_client_info() = CreateClientInfo(request_metadata); *feed_request.mutable_client_info() = CreateClientInfo(request_metadata);
feedwire::FeedQuery& query = *feed_request.mutable_feed_query(); feedwire::FeedQuery& query = *feed_request.mutable_feed_query();
query.set_reason(request_reason); query.set_reason(request_reason);
if (!consistency_token.empty()) {
// |consistency_token|, for action reporting, is only applicable to signed-in
// requests. The presence of |client_instance_id|, also signed-in only, can be
// used a proxy for checking if we're creating a signed-in request.
if (!consistency_token.empty() &&
!request_metadata.client_instance_id.empty()) {
feed_request.mutable_consistency_token()->set_token(consistency_token); feed_request.mutable_consistency_token()->set_token(consistency_token);
} }
if (!next_page_token.empty()) { if (!next_page_token.empty()) {
DCHECK_EQ(request_reason, feedwire::FeedQuery::NEXT_PAGE_SCROLL); DCHECK_EQ(request_reason, feedwire::FeedQuery::NEXT_PAGE_SCROLL);
query.mutable_next_page_token() query.mutable_next_page_token()
...@@ -187,7 +194,6 @@ bool CompareContent(const feedstore::Content& a, const feedstore::Content& b) { ...@@ -187,7 +194,6 @@ bool CompareContent(const feedstore::Content& a, const feedstore::Content& b) {
feedwire::ClientInfo CreateClientInfo(const RequestMetadata& request_metadata) { feedwire::ClientInfo CreateClientInfo(const RequestMetadata& request_metadata) {
feedwire::ClientInfo client_info; feedwire::ClientInfo client_info;
client_info.set_client_instance_id(request_metadata.client_instance_id);
feedwire::DisplayInfo& display_info = *client_info.add_display_info(); feedwire::DisplayInfo& display_info = *client_info.add_display_info();
display_info.set_screen_density(request_metadata.display_metrics.density); display_info.set_screen_density(request_metadata.display_metrics.density);
...@@ -207,6 +213,19 @@ feedwire::ClientInfo CreateClientInfo(const RequestMetadata& request_metadata) { ...@@ -207,6 +213,19 @@ feedwire::ClientInfo CreateClientInfo(const RequestMetadata& request_metadata) {
*client_info.mutable_platform_version() = GetPlatformVersionMessage(); *client_info.mutable_platform_version() = GetPlatformVersionMessage();
*client_info.mutable_app_version() = *client_info.mutable_app_version() =
GetAppVersionMessage(request_metadata.chrome_info); GetAppVersionMessage(request_metadata.chrome_info);
// client_instance_id and session_id should not both be set at the same time.
DCHECK(request_metadata.client_instance_id.empty() ||
request_metadata.session_id.empty());
// Populate client_instance_id, session_id, or neither.
if (!request_metadata.client_instance_id.empty()) {
client_info.set_client_instance_id(request_metadata.client_instance_id);
} else if (!request_metadata.session_id.empty()) {
client_info.mutable_chrome_client_info()->set_session_id(
request_metadata.session_id);
}
return client_info; return client_info;
} }
......
...@@ -306,6 +306,18 @@ RefreshResponseData TranslateWireResponse( ...@@ -306,6 +306,18 @@ RefreshResponseData TranslateWireResponse(
result->stream_data.set_privacy_notice_fulfilled( result->stream_data.set_privacy_notice_fulfilled(
response_metadata.privacy_notice_fulfilled()); response_metadata.privacy_notice_fulfilled());
base::Optional<std::string> session_id = base::nullopt;
if (was_signed_in_request) {
// Signed-in requests don't use session_id tokens; set an empty value to
// ensure that there are no old session_id tokens left hanging around.
session_id = std::string();
} else if (response_metadata.has_session_id()) {
// Signed-out requests can set a new session token; otherwise, we leave
// the default base::nullopt value to keep whatever token is already in
// play.
session_id = response_metadata.session_id();
}
MetricsReporter::ActivityLoggingEnabled(response_metadata.logging_enabled()); MetricsReporter::ActivityLoggingEnabled(response_metadata.logging_enabled());
MetricsReporter::NoticeCardFulfilledObsolete( MetricsReporter::NoticeCardFulfilledObsolete(
response_metadata.privacy_notice_fulfilled()); response_metadata.privacy_notice_fulfilled());
...@@ -313,6 +325,7 @@ RefreshResponseData TranslateWireResponse( ...@@ -313,6 +325,7 @@ RefreshResponseData TranslateWireResponse(
RefreshResponseData response_data; RefreshResponseData response_data;
response_data.model_update_request = std::move(result); response_data.model_update_request = std::move(result);
response_data.request_schedule = std::move(global_data.request_schedule); response_data.request_schedule = std::move(global_data.request_schedule);
response_data.session_id = std::move(session_id);
return response_data; return response_data;
} }
......
...@@ -64,6 +64,9 @@ struct RefreshResponseData { ...@@ -64,6 +64,9 @@ struct RefreshResponseData {
// Server-defined request schedule, if provided. // Server-defined request schedule, if provided.
base::Optional<RequestSchedule> request_schedule; base::Optional<RequestSchedule> request_schedule;
// Server-defined session id token, if provided.
base::Optional<std::string> session_id;
}; };
base::Optional<feedstore::DataOperation> TranslateDataOperation( base::Optional<feedstore::DataOperation> TranslateDataOperation(
......
...@@ -63,9 +63,14 @@ class FeedStreamApi { ...@@ -63,9 +63,14 @@ class FeedStreamApi {
// as the feed is refreshed or the user signs in/out. // as the feed is refreshed or the user signs in/out.
virtual bool IsActivityLoggingEnabled() const = 0; virtual bool IsActivityLoggingEnabled() const = 0;
// Returns the client_instance_id. This value is reset whenever the feed // Returns the signed-in client_instance_id. This value is reset whenever the
// stream is cleared (on sign-in, sign-out, and some data clear events). // feed stream is cleared (on sign-in, sign-out, and some data clear events).
virtual std::string GetClientInstanceId() = 0; virtual std::string GetClientInstanceId() const = 0;
// Returns the client's signed-out session id. This value is reset whenever
// the feed stream is cleared (on sign-in, sign-out, and some data clear
// events).
virtual std::string GetSessionId() const = 0;
// Invoked by RefreshTaskScheduler's scheduled task. // Invoked by RefreshTaskScheduler's scheduled task.
virtual void ExecuteRefreshTask() = 0; virtual void ExecuteRefreshTask() = 0;
......
...@@ -68,7 +68,7 @@ void LoadMoreTask::UploadActionsComplete(UploadActionsTask::Result result) { ...@@ -68,7 +68,7 @@ void LoadMoreTask::UploadActionsComplete(UploadActionsTask::Result result) {
fetch_start_time_ = stream_->GetTickClock()->NowTicks(); fetch_start_time_ = stream_->GetTickClock()->NowTicks();
stream_->GetNetwork()->SendQueryRequest( stream_->GetNetwork()->SendQueryRequest(
CreateFeedQueryLoadMoreRequest( CreateFeedQueryLoadMoreRequest(
stream_->GetRequestMetadata(), stream_->GetRequestMetadata(/*is_for_next_page=*/true),
stream_->GetMetadata()->GetConsistencyToken(), stream_->GetMetadata()->GetConsistencyToken(),
stream_->GetModel()->GetNextPageToken()), stream_->GetModel()->GetNextPageToken()),
force_signed_out_request, force_signed_out_request,
...@@ -94,6 +94,10 @@ void LoadMoreTask::QueryRequestComplete( ...@@ -94,6 +94,10 @@ void LoadMoreTask::QueryRequestComplete(
loaded_new_content_from_network_ = loaded_new_content_from_network_ =
!translated_response.model_update_request->stream_structures.empty(); !translated_response.model_update_request->stream_structures.empty();
stream_->GetMetadata()->MaybeUpdateSessionId(translated_response.session_id,
stream_->GetClock());
model->Update(std::move(translated_response.model_update_request)); model->Update(std::move(translated_response.model_update_request));
if (translated_response.request_schedule) if (translated_response.request_schedule)
......
...@@ -126,8 +126,6 @@ void LoadStreamFromStoreTask::Complete(LoadStreamStatus status) { ...@@ -126,8 +126,6 @@ void LoadStreamFromStoreTask::Complete(LoadStreamStatus status) {
if (status == LoadStreamStatus::kLoadedFromStore && if (status == LoadStreamStatus::kLoadedFromStore &&
load_type_ == LoadType::kFullLoad) { load_type_ == LoadType::kFullLoad) {
task_result.update_request = std::move(update_request_); task_result.update_request = std::move(update_request_);
} else {
task_result.consistency_token = consistency_token_;
} }
std::move(result_callback_).Run(std::move(task_result)); std::move(result_callback_).Run(std::move(task_result));
TaskComplete(); TaskComplete();
......
...@@ -76,7 +76,6 @@ class LoadStreamFromStoreTask : public offline_pages::Task { ...@@ -76,7 +76,6 @@ class LoadStreamFromStoreTask : public offline_pages::Task {
// Data to be stuffed into the Result when the task is complete. // Data to be stuffed into the Result when the task is complete.
std::unique_ptr<StreamModelUpdateRequest> update_request_; std::unique_ptr<StreamModelUpdateRequest> update_request_;
std::string consistency_token_;
std::vector<feedstore::StoredAction> pending_actions_; std::vector<feedstore::StoredAction> pending_actions_;
base::WeakPtrFactory<LoadStreamFromStoreTask> weak_ptr_factory_{this}; base::WeakPtrFactory<LoadStreamFromStoreTask> weak_ptr_factory_{this};
......
...@@ -123,7 +123,8 @@ void LoadStreamTask::UploadActionsComplete(UploadActionsTask::Result result) { ...@@ -123,7 +123,8 @@ void LoadStreamTask::UploadActionsComplete(UploadActionsTask::Result result) {
latencies_->StepComplete(LoadLatencyTimes::kUploadActions); latencies_->StepComplete(LoadLatencyTimes::kUploadActions);
stream_->GetNetwork()->SendQueryRequest( stream_->GetNetwork()->SendQueryRequest(
CreateFeedQueryRefreshRequest( CreateFeedQueryRefreshRequest(
GetRequestReason(load_type_), stream_->GetRequestMetadata(), GetRequestReason(load_type_),
stream_->GetRequestMetadata(/*is_for_next_page=*/false),
stream_->GetMetadata()->GetConsistencyToken()), stream_->GetMetadata()->GetConsistencyToken()),
force_signed_out_request, force_signed_out_request,
base::BindOnce(&LoadStreamTask::QueryRequestComplete, GetWeakPtr())); base::BindOnce(&LoadStreamTask::QueryRequestComplete, GetWeakPtr()));
...@@ -167,6 +168,9 @@ void LoadStreamTask::QueryRequestComplete( ...@@ -167,6 +168,9 @@ void LoadStreamTask::QueryRequestComplete(
stream_->SetLastStreamLoadHadNoticeCard(isNoticeCardFulfilled); stream_->SetLastStreamLoadHadNoticeCard(isNoticeCardFulfilled);
MetricsReporter::NoticeCardFulfilled(isNoticeCardFulfilled); MetricsReporter::NoticeCardFulfilled(isNoticeCardFulfilled);
stream_->GetMetadata()->MaybeUpdateSessionId(response_data.session_id,
stream_->GetClock());
if (load_type_ != LoadType::kBackgroundRefresh) { if (load_type_ != LoadType::kBackgroundRefresh) {
auto model = std::make_unique<StreamModel>(); auto model = std::make_unique<StreamModel>();
model->Update(std::move(response_data.model_update_request)); model->Update(std::move(response_data.model_update_request));
......
...@@ -158,12 +158,13 @@ std::vector<feedstore::DataOperation> MakeTypicalStreamOperations() { ...@@ -158,12 +158,13 @@ std::vector<feedstore::DataOperation> MakeTypicalStreamOperations() {
}; };
} }
std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState( StreamModelUpdateRequestGenerator::StreamModelUpdateRequestGenerator() =
int first_cluster_id, default;
base::Time last_added_time, StreamModelUpdateRequestGenerator::~StreamModelUpdateRequestGenerator() =
bool signed_in, default;
bool logging_enabled,
bool privacy_notice_fulfilled) { std::unique_ptr<StreamModelUpdateRequest>
StreamModelUpdateRequestGenerator::MakeFirstPage(int first_cluster_id) const {
auto initial_update = std::make_unique<StreamModelUpdateRequest>(); auto initial_update = std::make_unique<StreamModelUpdateRequest>();
const int i = first_cluster_id; const int i = first_cluster_id;
const int j = first_cluster_id + 1; const int j = first_cluster_id + 1;
...@@ -191,12 +192,8 @@ std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState( ...@@ -191,12 +192,8 @@ std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState(
return initial_update; return initial_update;
} }
std::unique_ptr<StreamModelUpdateRequest> MakeTypicalNextPageState( std::unique_ptr<StreamModelUpdateRequest>
int page_number, StreamModelUpdateRequestGenerator::MakeNextPage(int page_number) const {
base::Time last_added_time,
bool signed_in,
bool logging_enabled,
bool privacy_notice_fulfilled) {
auto initial_update = std::make_unique<StreamModelUpdateRequest>(); auto initial_update = std::make_unique<StreamModelUpdateRequest>();
initial_update->source = initial_update->source =
StreamModelUpdateRequest::Source::kInitialLoadFromStore; StreamModelUpdateRequest::Source::kInitialLoadFromStore;
...@@ -224,4 +221,32 @@ std::unique_ptr<StreamModelUpdateRequest> MakeTypicalNextPageState( ...@@ -224,4 +221,32 @@ std::unique_ptr<StreamModelUpdateRequest> MakeTypicalNextPageState(
return initial_update; return initial_update;
} }
std::unique_ptr<StreamModelUpdateRequest> MakeTypicalInitialModelState(
int first_cluster_id,
base::Time last_added_time,
bool signed_in,
bool logging_enabled,
bool privacy_notice_fulfilled) {
StreamModelUpdateRequestGenerator generator;
generator.last_added_time = last_added_time;
generator.signed_in = signed_in;
generator.logging_enabled = logging_enabled;
generator.privacy_notice_fulfilled = privacy_notice_fulfilled;
return generator.MakeFirstPage(first_cluster_id);
}
std::unique_ptr<StreamModelUpdateRequest> MakeTypicalNextPageState(
int page_number,
base::Time last_added_time,
bool signed_in,
bool logging_enabled,
bool privacy_notice_fulfilled) {
StreamModelUpdateRequestGenerator generator;
generator.last_added_time = last_added_time;
generator.signed_in = signed_in;
generator.logging_enabled = logging_enabled;
generator.privacy_notice_fulfilled = privacy_notice_fulfilled;
return generator.MakeNextPage(page_number);
}
} // namespace feed } // namespace feed
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/time/time.h" #include "base/time/time.h"
#include "components/feed/core/proto/v2/store.pb.h" #include "components/feed/core/proto/v2/store.pb.h"
#include "components/feed/core/v2/proto_util.h" #include "components/feed/core/v2/proto_util.h"
#include "components/feed/core/v2/protocol_translator.h"
#include "components/feed/core/v2/types.h" #include "components/feed/core/v2/types.h"
// Functions that help build a feedstore::StreamStructure for testing. // Functions that help build a feedstore::StreamStructure for testing.
...@@ -43,6 +44,25 @@ feedstore::Record MakeRecord( ...@@ -43,6 +44,25 @@ feedstore::Record MakeRecord(
feedstore::Record MakeRecord(feedstore::StreamSharedState shared_state); feedstore::Record MakeRecord(feedstore::StreamSharedState shared_state);
feedstore::Record MakeRecord(feedstore::StreamData stream_data); feedstore::Record MakeRecord(feedstore::StreamData stream_data);
// Helper structure to configure and return RefreshResponseData and
// StreamModelUpdateRequest objects denoting typical initial and next page
// refresh response payloads.
struct StreamModelUpdateRequestGenerator {
base::Time last_added_time = kTestTimeEpoch;
bool signed_in = true;
bool logging_enabled = true;
bool privacy_notice_fulfilled = true;
StreamModelUpdateRequestGenerator();
~StreamModelUpdateRequestGenerator();
std::unique_ptr<StreamModelUpdateRequest> MakeFirstPage(
int first_cluster_id = 0) const;
std::unique_ptr<StreamModelUpdateRequest> MakeNextPage(
int page_number = 2) const;
};
// Returns data operations to create a typical stream: // Returns data operations to create a typical stream:
// Root // Root
// |-Cluster 0 // |-Cluster 0
......
...@@ -39,6 +39,7 @@ struct RequestMetadata { ...@@ -39,6 +39,7 @@ struct RequestMetadata {
ChromeInfo chrome_info; ChromeInfo chrome_info;
std::string language_tag; std::string language_tag;
std::string client_instance_id; std::string client_instance_id;
std::string session_id;
DisplayMetrics display_metrics; DisplayMetrics display_metrics;
bool notice_card_acknowledged = false; bool notice_card_acknowledged = false;
}; };
......
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