Skip to content

Commit

Permalink
Update ServiceWorkerUpdateChecker to return sha256_checksums
Browse files Browse the repository at this point in the history
This CL will expose an option to return calculated sha256 checksums from
ServiceWorkerUpdateChecker. This internally uses the
ChecksumUpdateTiming::kAlways option in ServiceWorkerCacheWriter and
return checksums if the update check result is kIdentical.
ServiceWorkerUpdateChecker manages multiple checksums generated by each
script.

In this CL returned checksums are still not used, it will be handled by
the follow-up CL.

Bug: 1371756
Change-Id: I7472eb7191d28784680fe853477e1802d6b0bd5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4209712
Reviewed-by: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Commit-Queue: Shunya Shishido <sisidovski@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1102716}
  • Loading branch information
sisidovski authored and Chromium LUCI CQ committed Feb 8, 2023
1 parent 264b91a commit 4c6ed35
Show file tree
Hide file tree
Showing 14 changed files with 392 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -939,8 +939,7 @@ void ServiceWorkerCacheWriter::AsyncDoLoop(int result) {
}

std::string ServiceWorkerCacheWriter::GetSha256Checksum() {
DCHECK(STATE_DONE == state_ ||
checksum_update_timing_ == ChecksumUpdateTiming::kAlways);
DCHECK_EQ(STATE_DONE, state_);
DCHECK(checksum_);
uint8_t result[crypto::kSHA256Length];
checksum_->Finish(result, crypto::kSHA256Length);
Expand Down
4 changes: 4 additions & 0 deletions content/browser/service_worker/service_worker_cache_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ class CONTENT_EXPORT ServiceWorkerCacheWriter {
// of the checksum. It resets |checksum_| not to be called multiple times.
std::string GetSha256Checksum();

ChecksumUpdateTiming checksum_update_timing() const {
return checksum_update_timing_;
}

private:
class ReadResponseHeadCallbackAdapter;
class DataPipeReader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,8 @@ void ServiceWorkerNewScriptLoader::CommitCompleted(
CHECK_EQ(WriterState::kCompleted, body_writer_state_);
CHECK(cache_writer_->did_replace());
bytes_written = cache_writer_->bytes_written();
DCHECK_EQ(cache_writer_->checksum_update_timing(),
ServiceWorkerCacheWriter::ChecksumUpdateTiming::kCacheMismatch);
sha256_checksum = cache_writer_->GetSha256Checksum();
} else {
// When we fail a main script fetch, we do not have a renderer in which to
Expand Down
12 changes: 9 additions & 3 deletions content/browser/service_worker/service_worker_register_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,10 @@ bool ServiceWorkerRegisterJob::IsUpdateCheckNeeded() const {
void ServiceWorkerRegisterJob::OnUpdateCheckFinished(
ServiceWorkerSingleScriptUpdateChecker::Result result,
std::unique_ptr<ServiceWorkerSingleScriptUpdateChecker::FailureInfo>
failure_info) {
failure_info,
// TODO(crbug.com/1371756) `updated_sha256_script_checksums` will be used in
// a follow-up CL.
const std::map<GURL, std::string>& updated_sha256_script_checksums) {
// Update check failed.
if (result == ServiceWorkerSingleScriptUpdateChecker::Result::kFailed) {
DCHECK(failure_info);
Expand Down Expand Up @@ -622,10 +625,13 @@ void ServiceWorkerRegisterJob::UpdateAndContinue() {
int64_t script_resource_id =
version_to_update->script_cache_map()->LookupResourceId(script_url_);
DCHECK_NE(script_resource_id, blink::mojom::kInvalidServiceWorkerResourceId);
const absl::optional<std::string> script_sha256_chekcsum =
version_to_update->script_cache_map()->LookupSha256Checksum(script_url_);

update_checker_ = std::make_unique<ServiceWorkerUpdateChecker>(
std::move(resources), script_url_, script_resource_id, version_to_update,
std::move(loader_factory), force_bypass_cache_, worker_script_type_,
std::move(resources), script_url_, script_resource_id,
script_sha256_chekcsum, version_to_update, std::move(loader_factory),
force_bypass_cache_, worker_script_type_,
registration()->update_via_cache(), time_since_last_check, context_,
outside_fetch_client_settings_object_.Clone());
update_checker_->Start(
Expand Down
5 changes: 4 additions & 1 deletion content/browser/service_worker/service_worker_register_job.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@ class ServiceWorkerRegisterJob : public ServiceWorkerRegisterJobBase {
void OnUpdateCheckFinished(
ServiceWorkerSingleScriptUpdateChecker::Result result,
std::unique_ptr<ServiceWorkerSingleScriptUpdateChecker::FailureInfo>
failure_info);
failure_info,
// TODO(crbug.com/1371756): `updated_sha256_script_checksums` will be used
// in a follow-up CL.
const std::map<GURL, std::string>& updated_sha256_script_checksums);

void RegisterAndContinue();
void ContinueWithNewRegistration(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ int64_t ServiceWorkerScriptCacheMap::LookupResourceId(const GURL& url) {
return found->second->resource_id;
}

absl::optional<std::string> ServiceWorkerScriptCacheMap::LookupSha256Checksum(
const GURL& url) {
ResourceMap::const_iterator found = resource_map_.find(url);
if (found == resource_map_.end()) {
return absl::nullopt;
}
return found->second->sha256_checksum;
}

void ServiceWorkerScriptCacheMap::NotifyStartedCaching(const GURL& url,
int64_t resource_id) {
DCHECK_EQ(blink::mojom::kInvalidServiceWorkerResourceId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap {
delete;

int64_t LookupResourceId(const GURL& url);
absl::optional<std::string> LookupSha256Checksum(const GURL& url);

// Used during the initial run of a new version to build the map
// of resources ids.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ void ServiceWorkerScriptLoaderFactory::OnCopyScriptFinished(
}

int64_t resource_size = cache_writer_->bytes_written();
DCHECK_EQ(cache_writer_->checksum_update_timing(),
ServiceWorkerCacheWriter::ChecksumUpdateTiming::kCacheMismatch);
std::string sha256_checksum = cache_writer_->GetSha256Checksum();
cache_writer_.reset();
scoped_refptr<ServiceWorkerVersion> version = worker_host_->version();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,15 @@ ServiceWorkerSingleScriptUpdateChecker::ServiceWorkerSingleScriptUpdateChecker(
mojo::Remote<storage::mojom::ServiceWorkerResourceReader> copy_reader,
mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer,
int64_t writer_resource_id,
ScriptChecksumUpdateOption script_checksum_update_option,
ResultCallback callback)
: script_url_(script_url),
is_main_script_(is_main_script),
scope_(scope),
force_bypass_cache_(force_bypass_cache),
update_via_cache_(update_via_cache),
time_since_last_check_(time_since_last_check),
script_checksum_update_option_(script_checksum_update_option),
network_watcher_(FROM_HERE,
mojo::SimpleWatcher::ArmingPolicy::MANUAL,
base::SequencedTaskRunner::GetCurrentDefault()),
Expand Down Expand Up @@ -163,10 +165,22 @@ ServiceWorkerSingleScriptUpdateChecker::ServiceWorkerSingleScriptUpdateChecker(
resource_request.load_flags |= net::LOAD_VALIDATE_CACHE;
}

ServiceWorkerCacheWriter::ChecksumUpdateTiming checksum_update_timing;
switch (script_checksum_update_option_) {
case ScriptChecksumUpdateOption::kForceUpdate:
checksum_update_timing =
ServiceWorkerCacheWriter::ChecksumUpdateTiming::kAlways;
break;
case ScriptChecksumUpdateOption::kDefault:
checksum_update_timing =
ServiceWorkerCacheWriter::ChecksumUpdateTiming::kCacheMismatch;
break;
}

cache_writer_ = ServiceWorkerCacheWriter::CreateForComparison(
std::move(compare_reader), std::move(copy_reader), std::move(writer),
writer_resource_id, /*pause_when_not_identical=*/true,
ServiceWorkerCacheWriter::ChecksumUpdateTiming::kCacheMismatch);
checksum_update_timing);

// Service worker update checking doesn't have a relevant frame and tab, so
// that `web_contents_getter` returns nullptr and the frame id is set to
Expand Down Expand Up @@ -632,7 +646,8 @@ void ServiceWorkerSingleScriptUpdateChecker::Fail(
Finish(Result::kFailed,
/*paused_state=*/nullptr,
std::make_unique<FailureInfo>(status, error_message,
std::move(network_status)));
std::move(network_status)),
/*sha256_checksum=*/absl::nullopt);
}

void ServiceWorkerSingleScriptUpdateChecker::Succeed(
Expand All @@ -641,28 +656,51 @@ void ServiceWorkerSingleScriptUpdateChecker::Succeed(
TRACE_EVENT_WITH_FLOW1(
"ServiceWorker", "ServiceWorkerSingleScriptUpdateChecker::Succeed", this,
TRACE_EVENT_FLAG_FLOW_IN, "result", ResultToString(result));

DCHECK_NE(result, Result::kFailed);
Finish(result, std::move(paused_state), /*failure_info=*/nullptr);

// Get calculated sha256 checksum when below conditions are both satisfied:
// 1: |script_checksum_update_option_| is kForceUpdate.
// 2: |result| is kIdentical.
//
// When the result is kDifferent, |cache_writer_| doesn't scan all the data
// and it can be pausing. In this case, the finalized checksum is still not
// available here, and that will be handled in
// ServiceWorkerUpdatedScriptLoader.
absl::optional<std::string> sha256_checksum;
if (script_checksum_update_option_ ==
ScriptChecksumUpdateOption::kForceUpdate &&
result == Result::kIdentical) {
DCHECK(cache_writer_);
DCHECK_EQ(cache_writer_->checksum_update_timing(),
ServiceWorkerCacheWriter::ChecksumUpdateTiming::kAlways);
sha256_checksum = cache_writer_->GetSha256Checksum();
}

Finish(result, std::move(paused_state), /*failure_info=*/nullptr,
sha256_checksum);
}

void ServiceWorkerSingleScriptUpdateChecker::Finish(
Result result,
std::unique_ptr<PausedState> paused_state,
std::unique_ptr<FailureInfo> failure_info) {
std::unique_ptr<FailureInfo> failure_info,
const absl::optional<std::string>& sha256_checksum) {
network_watcher_.Cancel();
if (Result::kDifferent == result) {
DCHECK(paused_state);
// When the result if kDifferent, the checksum will be handled by
// ServiceWorkerUpdatedScriptLoader.
std::move(callback_).Run(script_url_, result, nullptr,
std::move(paused_state));
std::move(paused_state),
/*sha256_checksum=*/absl::nullopt);
return;
}

network_loader_.reset();
network_client_receiver_.reset();
network_consumer_.reset();
std::move(callback_).Run(script_url_, result, std::move(failure_info),
nullptr);
nullptr, sha256_checksum);
}

ServiceWorkerSingleScriptUpdateChecker::PausedState::PausedState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
kDifferent,
};

// These values indicate if the update check returns the updated sha256
// checksum from |cache_writer_|.
//
// kDefault: By default, this class doesn't handle the script sha256
// checksum. The checksum is updated only when there is a cache mismatch but
// this class never checks it.
// kForceUpdate: If this value is passed to the ctor of this class,
// the checksum is updated even when there is no cache mismatch, and the
// updated checksum is passed to the |ResultCallback| param only if the check
// result is |kIdentical|. If the result is |kDifferent| or others, the
// checksum wouldn't be passed.
enum class ScriptChecksumUpdateOption {
kDefault,
kForceUpdate,
};

// This contains detailed error info of update check when it failed.
struct CONTENT_EXPORT FailureInfo {
FailureInfo(blink::ServiceWorkerStatusCode status,
Expand Down Expand Up @@ -111,10 +127,12 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
// is transferred to the callback in the PausedState only when the result is
// Result::kDifferent. Otherwise it's set to nullptr. FailureInfo is set to a
// valid value if the result is Result::kFailed, otherwise it'll be nullptr.
using ResultCallback = base::OnceCallback<void(const GURL&,
Result,
std::unique_ptr<FailureInfo>,
std::unique_ptr<PausedState>)>;
using ResultCallback = base::OnceCallback<void(
const GURL&,
Result,
std::unique_ptr<FailureInfo>,
std::unique_ptr<PausedState>,
const absl::optional<std::string>& sha256_checksum)>;

ServiceWorkerSingleScriptUpdateChecker() = delete;

Expand All @@ -140,6 +158,7 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
mojo::Remote<storage::mojom::ServiceWorkerResourceReader> copy_reader,
mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer,
int64_t write_resource_id,
ScriptChecksumUpdateOption script_checksum_update_option,
ResultCallback callback);

ServiceWorkerSingleScriptUpdateChecker(
Expand Down Expand Up @@ -198,7 +217,8 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
void Succeed(Result result, std::unique_ptr<PausedState> paused_state);
void Finish(Result result,
std::unique_ptr<PausedState> paused_state,
std::unique_ptr<FailureInfo> failure_info);
std::unique_ptr<FailureInfo> failure_info,
const absl::optional<std::string>& sha256_checksum);

const GURL script_url_;
const bool is_main_script_;
Expand All @@ -207,6 +227,8 @@ class CONTENT_EXPORT ServiceWorkerSingleScriptUpdateChecker
const blink::mojom::ServiceWorkerUpdateViaCache update_via_cache_;
const base::TimeDelta time_since_last_check_;
bool network_accessed_ = false;
const ScriptChecksumUpdateOption script_checksum_update_option_ =
ScriptChecksumUpdateOption::kDefault;
scoped_refptr<PolicyContainerHost> policy_container_host_;

// The endpoint called by `network_loader_`. That needs to be alive while
Expand Down

0 comments on commit 4c6ed35

Please sign in to comment.