Commit 0f6438ff authored by Kenichi Ishibashi's avatar Kenichi Ishibashi Committed by Commit Bot

ServiceWorkerInstalledScriptReader should handle remote disconnection

ServiceWorkerInstalledScriptReader owns a ServiceWorkerResourceReader
which can be disconnected when the Storage Service disappears. Set
a disconnection handler that calls CompleteSendIfNeeded() to abort
script reading gracefully.

Bug: 1133143
Change-Id: I94b83798834f4287798df5e24e670309961af7cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2519682Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825246}
parent c42288d8
...@@ -94,12 +94,17 @@ class ServiceWorkerInstalledScriptReader::MetaDataSender { ...@@ -94,12 +94,17 @@ class ServiceWorkerInstalledScriptReader::MetaDataSender {
ServiceWorkerInstalledScriptReader::ServiceWorkerInstalledScriptReader( ServiceWorkerInstalledScriptReader::ServiceWorkerInstalledScriptReader(
mojo::Remote<storage::mojom::ServiceWorkerResourceReader> reader, mojo::Remote<storage::mojom::ServiceWorkerResourceReader> reader,
Client* client) Client* client)
: reader_(std::move(reader)), client_(client) {} : reader_(std::move(reader)), client_(client) {
DCHECK(reader_.is_connected());
reader_.set_disconnect_handler(base::BindOnce(
&ServiceWorkerInstalledScriptReader::OnReaderDisconnected, AsWeakPtr()));
}
ServiceWorkerInstalledScriptReader::~ServiceWorkerInstalledScriptReader() {} ServiceWorkerInstalledScriptReader::~ServiceWorkerInstalledScriptReader() {}
void ServiceWorkerInstalledScriptReader::Start() { void ServiceWorkerInstalledScriptReader::Start() {
TRACE_EVENT0("ServiceWorker", "ServiceWorkerInstalledScriptReader::Start"); TRACE_EVENT0("ServiceWorker", "ServiceWorkerInstalledScriptReader::Start");
DCHECK(reader_.is_connected());
reader_->ReadResponseHead(base::BindOnce( reader_->ReadResponseHead(base::BindOnce(
&ServiceWorkerInstalledScriptReader::OnReadResponseHeadComplete, &ServiceWorkerInstalledScriptReader::OnReadResponseHeadComplete,
AsWeakPtr())); AsWeakPtr()));
...@@ -122,6 +127,7 @@ void ServiceWorkerInstalledScriptReader::OnReadResponseHeadComplete( ...@@ -122,6 +127,7 @@ void ServiceWorkerInstalledScriptReader::OnReadResponseHeadComplete(
} }
DCHECK_GE(result, 0); DCHECK_GE(result, 0);
DCHECK(reader_.is_connected());
body_size_ = response_head->content_length; body_size_ = response_head->content_length;
int64_t content_length = response_head->content_length; int64_t content_length = response_head->content_length;
...@@ -176,6 +182,10 @@ void ServiceWorkerInstalledScriptReader::OnReadDataStarted( ...@@ -176,6 +182,10 @@ void ServiceWorkerInstalledScriptReader::OnReadDataStarted(
std::move(meta_data_consumer)); std::move(meta_data_consumer));
} }
void ServiceWorkerInstalledScriptReader::OnReaderDisconnected() {
CompleteSendIfNeeded(FinishedReason::kConnectionError);
}
void ServiceWorkerInstalledScriptReader::OnMetaDataSent(bool success) { void ServiceWorkerInstalledScriptReader::OnMetaDataSent(bool success) {
meta_data_sender_.reset(); meta_data_sender_.reset();
if (!success) { if (!success) {
......
...@@ -74,6 +74,7 @@ class ServiceWorkerInstalledScriptReader ...@@ -74,6 +74,7 @@ class ServiceWorkerInstalledScriptReader
base::Optional<mojo_base::BigBuffer> metadata, base::Optional<mojo_base::BigBuffer> metadata,
mojo::ScopedDataPipeConsumerHandle body_consumer_handle); mojo::ScopedDataPipeConsumerHandle body_consumer_handle);
void OnMetaDataSent(bool success); void OnMetaDataSent(bool success);
void OnReaderDisconnected();
void CompleteSendIfNeeded(FinishedReason reason); void CompleteSendIfNeeded(FinishedReason reason);
bool WasMetadataWritten() const { return !meta_data_sender_; } bool WasMetadataWritten() const { return !meta_data_sender_; }
bool WasBodyWritten() const { return was_body_written_; } bool WasBodyWritten() const { return was_body_written_; }
......
...@@ -148,6 +148,7 @@ class ServiceWorkerInstalledScriptsSenderTest : public testing::Test { ...@@ -148,6 +148,7 @@ class ServiceWorkerInstalledScriptsSenderTest : public testing::Test {
EmbeddedWorkerTestHelper* helper() { return helper_.get(); } EmbeddedWorkerTestHelper* helper() { return helper_.get(); }
ServiceWorkerContextCore* context() { return helper_->context(); } ServiceWorkerContextCore* context() { return helper_->context(); }
ServiceWorkerRegistry* registry() { return context()->registry(); }
ServiceWorkerVersion* version() { return version_.get(); } ServiceWorkerVersion* version() { return version_.get(); }
private: private:
...@@ -650,4 +651,64 @@ TEST_F(ServiceWorkerInstalledScriptsSenderTest, NoContext) { ...@@ -650,4 +651,64 @@ TEST_F(ServiceWorkerInstalledScriptsSenderTest, NoContext) {
EXPECT_EQ(sender->last_finished_reason(), FinishedReason::kNoContextError); EXPECT_EQ(sender->last_finished_reason(), FinishedReason::kNoContextError);
} }
// Test that the scripts sender aborts gracefully when a remote connection to
// the Storage Service is disconnected.
TEST_F(ServiceWorkerInstalledScriptsSenderTest, RemoteStorageDisconnection) {
const GURL kMainScriptURL = version()->script_url();
std::map<GURL, ExpectedScriptInfo> kExpectedScriptInfoMap = {
{kMainScriptURL,
{1,
kMainScriptURL,
{{"Content-Length", "35"},
{"Content-Type", "text/javascript; charset=utf-8"},
{"TestHeader", "BlahBlah"}},
"utf-8",
"I'm script body for the main script",
"I'm meta data for the main script"}}};
std::vector<storage::mojom::ServiceWorkerResourceRecordPtr> records;
for (const auto& info : kExpectedScriptInfoMap)
records.push_back(
info.second.WriteToDiskCache(context()->GetStorageControl()));
version()->script_cache_map()->SetResources(records);
auto sender =
std::make_unique<ServiceWorkerInstalledScriptsSender>(version());
sender->Start();
registry()->SimulateStorageRestartForTesting();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(sender->last_finished_reason(), FinishedReason::kConnectionError);
}
// Test that the scripts sender aborts gracefully when the storage is disabled.
TEST_F(ServiceWorkerInstalledScriptsSenderTest, StorageDisabled) {
const GURL kMainScriptURL = version()->script_url();
std::map<GURL, ExpectedScriptInfo> kExpectedScriptInfoMap = {
{kMainScriptURL,
{1,
kMainScriptURL,
{{"Content-Length", "35"},
{"Content-Type", "text/javascript; charset=utf-8"},
{"TestHeader", "BlahBlah"}},
"utf-8",
"I'm script body for the main script",
"I'm meta data for the main script"}}};
std::vector<storage::mojom::ServiceWorkerResourceRecordPtr> records;
for (const auto& info : kExpectedScriptInfoMap)
records.push_back(
info.second.WriteToDiskCache(context()->GetStorageControl()));
version()->script_cache_map()->SetResources(records);
registry()->GetRemoteStorageControl()->Disable();
registry()->GetRemoteStorageControl().FlushForTesting();
auto sender =
std::make_unique<ServiceWorkerInstalledScriptsSender>(version());
sender->Start();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(sender->last_finished_reason(), FinishedReason::kConnectionError);
}
} // namespace content } // namespace content
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