Commit 334e05fb authored by iclelland's avatar iclelland Committed by Commit Bot

Deflake origin trial token validator tests

This removes the dependency on the system clock of the origin trial
token validator tests. All static functions in the TrialTokenValidator
namespace which validate tokens now take a base::Time argument which is used to
compare against the tokens provided.

BUG=672294
R=chasej@chromium.org,nhiroki@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_n5x_swarming_rel

Review-Url: https://codereview.chromium.org/2555333006
Cr-Commit-Position: refs/heads/master@{#491601}
parent 7687d9be
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/test/simple_test_clock.h"
#include "base/test/simple_test_tick_clock.h" #include "base/test/simple_test_tick_clock.h"
#include "content/browser/browser_thread_impl.h" #include "content/browser/browser_thread_impl.h"
#include "content/browser/service_worker/embedded_worker_test_helper.h" #include "content/browser/service_worker/embedded_worker_test_helper.h"
...@@ -50,6 +51,10 @@ int kMockProviderId = 1; ...@@ -50,6 +51,10 @@ int kMockProviderId = 1;
const char* kValidUrl = "https://valid.example.com/foo/bar"; const char* kValidUrl = "https://valid.example.com/foo/bar";
// This timestamp is set to a time after the expiry timestamp of the expired
// tokens in this test, but before the expiry timestamp of the valid ones.
double kNowTimestamp = 1500000000;
void EmptyCallback() {} void EmptyCallback() {}
} // namespace } // namespace
...@@ -79,6 +84,12 @@ class ForeignFetchRequestHandlerTest : public testing::Test { ...@@ -79,6 +84,12 @@ class ForeignFetchRequestHandlerTest : public testing::Test {
kVersionId, context()->AsWeakPtr()); kVersionId, context()->AsWeakPtr());
version_->set_foreign_fetch_scopes({kScope}); version_->set_foreign_fetch_scopes({kScope});
// Fix the time for testing to kNowTimestamp
std::unique_ptr<base::SimpleTestClock> clock =
base::MakeUnique<base::SimpleTestClock>();
clock->SetNow(base::Time::FromDoubleT(kNowTimestamp));
version_->SetClockForTesting(std::move(clock));
context()->storage()->LazyInitialize(base::Bind(&EmptyCallback)); context()->storage()->LazyInitialize(base::Bind(&EmptyCallback));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/time/time.h"
#include "components/link_header_util/link_header_util.h" #include "components/link_header_util/link_header_util.h"
#include "content/browser/loader/resource_message_filter.h" #include "content/browser/loader/resource_message_filter.h"
#include "content/browser/loader/resource_request_info_impl.h" #include "content/browser/loader/resource_request_info_impl.h"
...@@ -42,7 +43,8 @@ void HandleServiceWorkerLink( ...@@ -42,7 +43,8 @@ void HandleServiceWorkerLink(
if (!base::CommandLine::ForCurrentProcess()->HasSwitch( if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableExperimentalWebPlatformFeatures) && switches::kEnableExperimentalWebPlatformFeatures) &&
!TrialTokenValidator::RequestEnablesFeature(request, "ForeignFetch")) { !TrialTokenValidator::RequestEnablesFeature(request, "ForeignFetch",
base::Time::Now())) {
// TODO(mek): Log attempt to use without having correct token? // TODO(mek): Log attempt to use without having correct token?
return; return;
} }
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/time/default_clock.h"
#include "base/time/default_tick_clock.h" #include "base/time/default_tick_clock.h"
#include "content/browser/bad_message.h" #include "content/browser/bad_message.h"
#include "content/browser/child_process_security_policy_impl.h" #include "content/browser/child_process_security_policy_impl.h"
...@@ -290,7 +291,8 @@ ServiceWorkerVersion::ServiceWorkerVersion( ...@@ -290,7 +291,8 @@ ServiceWorkerVersion::ServiceWorkerVersion(
site_for_uma_(ServiceWorkerMetrics::SiteFromURL(scope_)), site_for_uma_(ServiceWorkerMetrics::SiteFromURL(scope_)),
context_(context), context_(context),
script_cache_map_(this, context), script_cache_map_(this, context),
tick_clock_(base::WrapUnique(new base::DefaultTickClock)), tick_clock_(base::MakeUnique<base::DefaultTickClock>()),
clock_(base::MakeUnique<base::DefaultClock>()),
ping_controller_(new PingController(this)), ping_controller_(new PingController(this)),
weak_factory_(this) { weak_factory_(this) {
DCHECK_NE(kInvalidServiceWorkerVersionId, version_id); DCHECK_NE(kInvalidServiceWorkerVersionId, version_id);
...@@ -552,7 +554,7 @@ int ServiceWorkerVersion::StartRequestWithCustomTimeout( ...@@ -552,7 +554,7 @@ int ServiceWorkerVersion::StartRequestWithCustomTimeout(
<< " can only be dispatched to an active worker: " << status(); << " can only be dispatched to an active worker: " << status();
int request_id = pending_requests_.Add(base::MakeUnique<PendingRequest>( int request_id = pending_requests_.Add(base::MakeUnique<PendingRequest>(
error_callback, base::Time::Now(), tick_clock_->NowTicks(), event_type)); error_callback, clock_->Now(), tick_clock_->NowTicks(), event_type));
TRACE_EVENT_ASYNC_BEGIN2("ServiceWorker", "ServiceWorkerVersion::Request", TRACE_EVENT_ASYNC_BEGIN2("ServiceWorker", "ServiceWorkerVersion::Request",
pending_requests_.Lookup(request_id), "Request id", pending_requests_.Lookup(request_id), "Request id",
request_id, "Event type", request_id, "Event type",
...@@ -622,7 +624,7 @@ bool ServiceWorkerVersion::FinishExternalRequest( ...@@ -622,7 +624,7 @@ bool ServiceWorkerVersion::FinishExternalRequest(
if (iter != external_request_uuid_to_request_id_.end()) { if (iter != external_request_uuid_to_request_id_.end()) {
int request_id = iter->second; int request_id = iter->second;
external_request_uuid_to_request_id_.erase(iter); external_request_uuid_to_request_id_.erase(iter);
return FinishRequest(request_id, true, base::Time::Now()); return FinishRequest(request_id, true, clock_->Now());
} }
// It is possible that the request was cancelled or timed out before and we // It is possible that the request was cancelled or timed out before and we
...@@ -743,8 +745,8 @@ void ServiceWorkerVersion::Doom() { ...@@ -743,8 +745,8 @@ void ServiceWorkerVersion::Doom() {
void ServiceWorkerVersion::SetValidOriginTrialTokens( void ServiceWorkerVersion::SetValidOriginTrialTokens(
const TrialTokenValidator::FeatureToTokensMap& tokens) { const TrialTokenValidator::FeatureToTokensMap& tokens) {
origin_trial_tokens_ = origin_trial_tokens_ = TrialTokenValidator::GetValidTokens(
TrialTokenValidator::GetValidTokens(url::Origin(scope()), tokens); url::Origin(scope()), tokens, clock_->Now());
} }
void ServiceWorkerVersion::SetDevToolsAttached(bool attached) { void ServiceWorkerVersion::SetDevToolsAttached(bool attached) {
...@@ -796,7 +798,7 @@ void ServiceWorkerVersion::SetMainScriptHttpResponseInfo( ...@@ -796,7 +798,7 @@ void ServiceWorkerVersion::SetMainScriptHttpResponseInfo(
// wasn't set in the entry. // wasn't set in the entry.
if (!origin_trial_tokens_) { if (!origin_trial_tokens_) {
origin_trial_tokens_ = TrialTokenValidator::GetValidTokensFromHeaders( origin_trial_tokens_ = TrialTokenValidator::GetValidTokensFromHeaders(
url::Origin(scope()), http_info.headers.get()); url::Origin(scope()), http_info.headers.get(), clock_->Now());
} }
for (auto& observer : listeners_) for (auto& observer : listeners_)
...@@ -812,6 +814,11 @@ void ServiceWorkerVersion::SetTickClockForTesting( ...@@ -812,6 +814,11 @@ void ServiceWorkerVersion::SetTickClockForTesting(
tick_clock_ = std::move(tick_clock); tick_clock_ = std::move(tick_clock);
} }
void ServiceWorkerVersion::SetClockForTesting(
std::unique_ptr<base::Clock> clock) {
clock_ = std::move(clock);
}
const net::HttpResponseInfo* const net::HttpResponseInfo*
ServiceWorkerVersion::GetMainScriptHttpResponseInfo() { ServiceWorkerVersion::GetMainScriptHttpResponseInfo() {
return main_script_http_info_.get(); return main_script_http_info_.get();
...@@ -1799,7 +1806,7 @@ void ServiceWorkerVersion::MarkIfStale() { ...@@ -1799,7 +1806,7 @@ void ServiceWorkerVersion::MarkIfStale() {
if (!registration || registration->active_version() != this) if (!registration || registration->active_version() != this)
return; return;
base::TimeDelta time_since_last_check = base::TimeDelta time_since_last_check =
base::Time::Now() - registration->last_update_check(); clock_->Now() - registration->last_update_check();
if (time_since_last_check > kServiceWorkerScriptMaxCacheAge) if (time_since_last_check > kServiceWorkerScriptMaxCacheAge)
RestartTick(&stale_time_); RestartTick(&stale_time_);
} }
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "base/observer_list.h" #include "base/observer_list.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/time/clock.h"
#include "base/time/tick_clock.h" #include "base/time/tick_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
...@@ -392,6 +393,9 @@ class CONTENT_EXPORT ServiceWorkerVersion ...@@ -392,6 +393,9 @@ class CONTENT_EXPORT ServiceWorkerVersion
// Used to allow tests to change time for testing. // Used to allow tests to change time for testing.
void SetTickClockForTesting(std::unique_ptr<base::TickClock> tick_clock); void SetTickClockForTesting(std::unique_ptr<base::TickClock> tick_clock);
// Used to allow tests to change wall clock for testing.
void SetClockForTesting(std::unique_ptr<base::Clock> clock);
// Returns true if the service worker has work to do: it has pending // Returns true if the service worker has work to do: it has pending
// requests, in-progress streaming URLRequestJobs, or pending start callbacks. // requests, in-progress streaming URLRequestJobs, or pending start callbacks.
bool HasWork() const; bool HasWork() const;
...@@ -794,6 +798,9 @@ class CONTENT_EXPORT ServiceWorkerVersion ...@@ -794,6 +798,9 @@ class CONTENT_EXPORT ServiceWorkerVersion
// The clock used to vend tick time. // The clock used to vend tick time.
std::unique_ptr<base::TickClock> tick_clock_; std::unique_ptr<base::TickClock> tick_clock_;
// The clock used for actual (wall clock) time
std::unique_ptr<base::Clock> clock_;
std::unique_ptr<PingController> ping_controller_; std::unique_ptr<PingController> ping_controller_;
// Used for recording worker activities (e.g., a ratio of handled events) // Used for recording worker activities (e.g., a ratio of handled events)
......
...@@ -21,7 +21,8 @@ namespace content { ...@@ -21,7 +21,8 @@ namespace content {
blink::WebOriginTrialTokenStatus TrialTokenValidator::ValidateToken( blink::WebOriginTrialTokenStatus TrialTokenValidator::ValidateToken(
const std::string& token, const std::string& token,
const url::Origin& origin, const url::Origin& origin,
std::string* feature_name) { std::string* feature_name,
base::Time current_time) {
ContentClient* content_client = GetContentClient(); ContentClient* content_client = GetContentClient();
const OriginTrialPolicy* origin_trial_policy = const OriginTrialPolicy* origin_trial_policy =
content_client->GetOriginTrialPolicy(); content_client->GetOriginTrialPolicy();
...@@ -40,7 +41,7 @@ blink::WebOriginTrialTokenStatus TrialTokenValidator::ValidateToken( ...@@ -40,7 +41,7 @@ blink::WebOriginTrialTokenStatus TrialTokenValidator::ValidateToken(
if (status != blink::WebOriginTrialTokenStatus::kSuccess) if (status != blink::WebOriginTrialTokenStatus::kSuccess)
return status; return status;
status = trial_token->IsValid(origin, base::Time::Now()); status = trial_token->IsValid(origin, current_time);
if (status != blink::WebOriginTrialTokenStatus::kSuccess) if (status != blink::WebOriginTrialTokenStatus::kSuccess)
return status; return status;
...@@ -54,19 +55,20 @@ blink::WebOriginTrialTokenStatus TrialTokenValidator::ValidateToken( ...@@ -54,19 +55,20 @@ blink::WebOriginTrialTokenStatus TrialTokenValidator::ValidateToken(
return blink::WebOriginTrialTokenStatus::kSuccess; return blink::WebOriginTrialTokenStatus::kSuccess;
} }
bool TrialTokenValidator::RequestEnablesFeature( bool TrialTokenValidator::RequestEnablesFeature(const net::URLRequest* request,
const net::URLRequest* request, base::StringPiece feature_name,
base::StringPiece feature_name) { base::Time current_time) {
// TODO(mek): Possibly cache the features that are availble for request in // TODO(mek): Possibly cache the features that are availble for request in
// UserData associated with the request. // UserData associated with the request.
return RequestEnablesFeature(request->url(), request->response_headers(), return RequestEnablesFeature(request->url(), request->response_headers(),
feature_name); feature_name, current_time);
} }
bool TrialTokenValidator::RequestEnablesFeature( bool TrialTokenValidator::RequestEnablesFeature(
const GURL& request_url, const GURL& request_url,
const net::HttpResponseHeaders* response_headers, const net::HttpResponseHeaders* response_headers,
base::StringPiece feature_name) { base::StringPiece feature_name,
base::Time current_time) {
if (!base::FeatureList::IsEnabled(features::kOriginTrials)) if (!base::FeatureList::IsEnabled(features::kOriginTrials))
return false; return false;
...@@ -79,7 +81,7 @@ bool TrialTokenValidator::RequestEnablesFeature( ...@@ -79,7 +81,7 @@ bool TrialTokenValidator::RequestEnablesFeature(
while (response_headers->EnumerateHeader(&iter, "Origin-Trial", &token)) { while (response_headers->EnumerateHeader(&iter, "Origin-Trial", &token)) {
std::string token_feature; std::string token_feature;
// TODO(mek): Log the validation errors to histograms? // TODO(mek): Log the validation errors to histograms?
if (ValidateToken(token, origin, &token_feature) == if (ValidateToken(token, origin, &token_feature, current_time) ==
blink::WebOriginTrialTokenStatus::kSuccess) blink::WebOriginTrialTokenStatus::kSuccess)
if (token_feature == feature_name) if (token_feature == feature_name)
return true; return true;
...@@ -90,7 +92,8 @@ bool TrialTokenValidator::RequestEnablesFeature( ...@@ -90,7 +92,8 @@ bool TrialTokenValidator::RequestEnablesFeature(
std::unique_ptr<TrialTokenValidator::FeatureToTokensMap> std::unique_ptr<TrialTokenValidator::FeatureToTokensMap>
TrialTokenValidator::GetValidTokensFromHeaders( TrialTokenValidator::GetValidTokensFromHeaders(
const url::Origin& origin, const url::Origin& origin,
const net::HttpResponseHeaders* headers) { const net::HttpResponseHeaders* headers,
base::Time current_time) {
std::unique_ptr<FeatureToTokensMap> tokens( std::unique_ptr<FeatureToTokensMap> tokens(
base::MakeUnique<FeatureToTokensMap>()); base::MakeUnique<FeatureToTokensMap>());
if (!base::FeatureList::IsEnabled(features::kOriginTrials)) if (!base::FeatureList::IsEnabled(features::kOriginTrials))
...@@ -103,7 +106,8 @@ TrialTokenValidator::GetValidTokensFromHeaders( ...@@ -103,7 +106,8 @@ TrialTokenValidator::GetValidTokensFromHeaders(
std::string token; std::string token;
while (headers->EnumerateHeader(&iter, "Origin-Trial", &token)) { while (headers->EnumerateHeader(&iter, "Origin-Trial", &token)) {
std::string token_feature; std::string token_feature;
if (TrialTokenValidator::ValidateToken(token, origin, &token_feature) == if (TrialTokenValidator::ValidateToken(token, origin, &token_feature,
current_time) ==
blink::WebOriginTrialTokenStatus::kSuccess) { blink::WebOriginTrialTokenStatus::kSuccess) {
(*tokens)[token_feature].push_back(token); (*tokens)[token_feature].push_back(token);
} }
...@@ -113,7 +117,8 @@ TrialTokenValidator::GetValidTokensFromHeaders( ...@@ -113,7 +117,8 @@ TrialTokenValidator::GetValidTokensFromHeaders(
std::unique_ptr<TrialTokenValidator::FeatureToTokensMap> std::unique_ptr<TrialTokenValidator::FeatureToTokensMap>
TrialTokenValidator::GetValidTokens(const url::Origin& origin, TrialTokenValidator::GetValidTokens(const url::Origin& origin,
const FeatureToTokensMap& tokens) { const FeatureToTokensMap& tokens,
base::Time current_time) {
std::unique_ptr<FeatureToTokensMap> out_tokens( std::unique_ptr<FeatureToTokensMap> out_tokens(
base::MakeUnique<FeatureToTokensMap>()); base::MakeUnique<FeatureToTokensMap>());
if (!base::FeatureList::IsEnabled(features::kOriginTrials)) if (!base::FeatureList::IsEnabled(features::kOriginTrials))
...@@ -125,7 +130,8 @@ TrialTokenValidator::GetValidTokens(const url::Origin& origin, ...@@ -125,7 +130,8 @@ TrialTokenValidator::GetValidTokens(const url::Origin& origin,
for (const auto& feature : tokens) { for (const auto& feature : tokens) {
for (const std::string& token : feature.second) { for (const std::string& token : feature.second) {
std::string token_feature; std::string token_feature;
if (TrialTokenValidator::ValidateToken(token, origin, &token_feature) == if (TrialTokenValidator::ValidateToken(token, origin, &token_feature,
current_time) ==
blink::WebOriginTrialTokenStatus::kSuccess) { blink::WebOriginTrialTokenStatus::kSuccess) {
DCHECK_EQ(token_feature, feature.first); DCHECK_EQ(token_feature, feature.first);
(*out_tokens)[feature.first].push_back(token); (*out_tokens)[feature.first].push_back(token);
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "base/time/time.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -35,26 +36,31 @@ using FeatureToTokensMap = std::map<std::string /* feature_name */, ...@@ -35,26 +36,31 @@ using FeatureToTokensMap = std::map<std::string /* feature_name */,
CONTENT_EXPORT blink::WebOriginTrialTokenStatus ValidateToken( CONTENT_EXPORT blink::WebOriginTrialTokenStatus ValidateToken(
const std::string& token, const std::string& token,
const url::Origin& origin, const url::Origin& origin,
std::string* feature_name); std::string* feature_name,
base::Time current_time);
CONTENT_EXPORT bool RequestEnablesFeature(const net::URLRequest* request, CONTENT_EXPORT bool RequestEnablesFeature(const net::URLRequest* request,
base::StringPiece feature_name); base::StringPiece feature_name,
base::Time current_time);
CONTENT_EXPORT bool RequestEnablesFeature( CONTENT_EXPORT bool RequestEnablesFeature(
const GURL& request_url, const GURL& request_url,
const net::HttpResponseHeaders* response_headers, const net::HttpResponseHeaders* response_headers,
base::StringPiece feature_name); base::StringPiece feature_name,
base::Time current_time);
// Returns all valid tokens in |headers|. // Returns all valid tokens in |headers|.
CONTENT_EXPORT std::unique_ptr<FeatureToTokensMap> GetValidTokensFromHeaders( CONTENT_EXPORT std::unique_ptr<FeatureToTokensMap> GetValidTokensFromHeaders(
const url::Origin& origin, const url::Origin& origin,
const net::HttpResponseHeaders* headers); const net::HttpResponseHeaders* headers,
base::Time current_time);
// Returns all valid tokens in |tokens|. This method is used to re-validate // Returns all valid tokens in |tokens|. This method is used to re-validate
// previously stored tokens. // previously stored tokens.
CONTENT_EXPORT std::unique_ptr<FeatureToTokensMap> GetValidTokens( CONTENT_EXPORT std::unique_ptr<FeatureToTokensMap> GetValidTokens(
const url::Origin& origin, const url::Origin& origin,
const FeatureToTokensMap& tokens); const FeatureToTokensMap& tokens,
base::Time current_time);
} // namespace TrialTokenValidator } // namespace TrialTokenValidator
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "content/renderer/origin_trials/web_trial_token_validator_impl.h" #include "content/renderer/origin_trials/web_trial_token_validator_impl.h"
#include "base/time/time.h"
#include "content/common/origin_trials/trial_token_validator.h" #include "content/common/origin_trials/trial_token_validator.h"
#include "third_party/WebKit/public/platform/WebOriginTrialTokenStatus.h" #include "third_party/WebKit/public/platform/WebOriginTrialTokenStatus.h"
...@@ -17,8 +18,8 @@ blink::WebOriginTrialTokenStatus WebTrialTokenValidatorImpl::ValidateToken( ...@@ -17,8 +18,8 @@ blink::WebOriginTrialTokenStatus WebTrialTokenValidatorImpl::ValidateToken(
const blink::WebSecurityOrigin& origin, const blink::WebSecurityOrigin& origin,
blink::WebString* feature_name) { blink::WebString* feature_name) {
std::string feature; std::string feature;
blink::WebOriginTrialTokenStatus status = blink::WebOriginTrialTokenStatus status = TrialTokenValidator::ValidateToken(
TrialTokenValidator::ValidateToken(token.Utf8(), origin, &feature); token.Utf8(), origin, &feature, base::Time::Now());
if (status == blink::WebOriginTrialTokenStatus::kSuccess) if (status == blink::WebOriginTrialTokenStatus::kSuccess)
*feature_name = blink::WebString::FromUTF8(feature); *feature_name = blink::WebString::FromUTF8(feature);
return status; return status;
......
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