Remove direct use of old service worker response writers
This CL replaces old writers with new mojo-based writers in tests.
Bug: 1055677
Change-Id: I68959e056092d2b6560067002b8ccfe466d8010a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2306113
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790156}
diff --git a/content/browser/service_worker/service_worker_job_unittest.cc b/content/browser/service_worker/service_worker_job_unittest.cc
index f3dcbc4..d9fdad2 100644
--- a/content/browser/service_worker/service_worker_job_unittest.cc
+++ b/content/browser/service_worker/service_worker_job_unittest.cc
@@ -13,6 +13,7 @@
#include "base/optional.h"
#include "base/run_loop.h"
#include "base/stl_util.h"
+#include "base/test/bind_test_util.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_simple_task_runner.h"
@@ -1171,18 +1172,11 @@
const char kBody[] = "/* old body */";
const char kNewBody[] = "/* new body */";
-void RunNestedUntilIdle() {
- base::RunLoop(base::RunLoop::Type::kNestableTasksAllowed).RunUntilIdle();
-}
-
-void OnIOComplete(int* rv_out, int rv) {
- *rv_out = rv;
-}
-
-void WriteResponse(ServiceWorkerResponseWriter* writer,
- const std::string& headers,
- IOBuffer* body,
- int length) {
+void WriteResponse(
+ mojo::Remote<storage::mojom::ServiceWorkerResourceWriter>& writer,
+ const std::string& headers,
+ mojo_base::BigBuffer body) {
+ int length = body.size();
auto response_head = network::mojom::URLResponseHead::New();
response_head->request_time = base::Time::Now();
response_head->response_time = base::Time::Now();
@@ -1190,24 +1184,37 @@
response_head->content_length = length;
int rv = -1234;
- writer->WriteResponseHead(*response_head, length,
- base::BindOnce(&OnIOComplete, &rv));
- RunNestedUntilIdle();
- EXPECT_LT(0, rv);
+ {
+ base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
+ writer->WriteResponseHead(std::move(response_head),
+ base::BindLambdaForTesting([&](int result) {
+ rv = result;
+ loop.Quit();
+ }));
+ loop.Run();
+ EXPECT_LT(0, rv);
+ }
rv = -1234;
- writer->WriteData(body, length, base::BindOnce(&OnIOComplete, &rv));
- RunNestedUntilIdle();
- EXPECT_EQ(length, rv);
+ {
+ base::RunLoop loop(base::RunLoop::Type::kNestableTasksAllowed);
+ writer->WriteData(std::move(body),
+ base::BindLambdaForTesting([&](int result) {
+ rv = result;
+ loop.Quit();
+ }));
+ loop.Run();
+ EXPECT_EQ(length, rv);
+ }
}
-void WriteStringResponse(ServiceWorkerResponseWriter* writer,
- const std::string& body) {
- scoped_refptr<IOBuffer> body_buffer =
- base::MakeRefCounted<WrappedIOBuffer>(body.data());
+void WriteStringResponse(
+ mojo::Remote<storage::mojom::ServiceWorkerResourceWriter>& writer,
+ const std::string& body) {
+ mojo_base::BigBuffer body_buffer(base::as_bytes(base::make_span(body)));
const char kHttpHeaders[] = "HTTP/1.0 200 HONKYDORY\0\0";
std::string headers(kHttpHeaders, base::size(kHttpHeaders));
- WriteResponse(writer, headers, body_buffer.get(), body.length());
+ WriteResponse(writer, headers, std::move(body_buffer));
}
class UpdateJobTestHelper : public EmbeddedWorkerTestHelper,
@@ -1322,6 +1329,14 @@
// EmbeddedWorkerTestHelper overrides:
void PopulateScriptCacheMap(int64_t version_id,
base::OnceClosure callback) override {
+ context()->GetStorageControl()->GetNewResourceId(base::BindOnce(
+ &UpdateJobTestHelper::DidGetNewResourceIdForScriptCache,
+ weak_factory_.GetWeakPtr(), version_id, std::move(callback)));
+ }
+
+ void DidGetNewResourceIdForScriptCache(int64_t version_id,
+ base::OnceClosure callback,
+ int64_t resource_id) {
ServiceWorkerVersion* version = context()->GetLiveVersion(version_id);
ASSERT_TRUE(version);
ServiceWorkerRegistration* registration =
@@ -1331,19 +1346,19 @@
bool is_update = registration->active_version() &&
version != registration->active_version();
- std::unique_ptr<ServiceWorkerResponseWriter> writer =
- CreateNewResponseWriterSync(storage());
- int64_t resource_id = writer->response_id();
+ mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer;
+ context()->GetStorageControl()->CreateResourceWriter(
+ resource_id, writer.BindNewPipeAndPassReceiver());
version->script_cache_map()->NotifyStartedCaching(script, resource_id);
if (!is_update) {
// Spoof caching the script for the initial version.
- WriteStringResponse(writer.get(), kBody);
+ WriteStringResponse(writer, kBody);
version->script_cache_map()->NotifyFinishedCaching(
script, base::size(kBody), net::OK, std::string());
} else {
EXPECT_NE(GURL(kNoChangeOrigin), script.GetOrigin());
// The script must be changed.
- WriteStringResponse(writer.get(), kNewBody);
+ WriteStringResponse(writer, kNewBody);
version->script_cache_map()->NotifyFinishedCaching(
script, base::size(kNewBody), net::OK, std::string());
}
@@ -1750,10 +1765,11 @@
// Make sure the storage has the data of the current waiting version.
const int64_t resource_id = 2;
- std::unique_ptr<ServiceWorkerResponseWriter> writer =
- storage()->CreateResponseWriter(resource_id);
+ mojo::Remote<storage::mojom::ServiceWorkerResourceWriter> writer;
+ context()->GetStorageControl()->CreateResourceWriter(
+ resource_id, writer.BindNewPipeAndPassReceiver());
version->script_cache_map()->NotifyStartedCaching(new_script, resource_id);
- WriteStringResponse(writer.get(), kBody);
+ WriteStringResponse(writer, kBody);
version->script_cache_map()->NotifyFinishedCaching(
new_script, base::size(kBody), net::OK, std::string());
diff --git a/content/browser/service_worker/service_worker_new_script_loader_unittest.cc b/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
index 46c0105a..78888af 100644
--- a/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
+++ b/content/browser/service_worker/service_worker_new_script_loader_unittest.cc
@@ -218,7 +218,7 @@
int routing_id = 0;
int request_id = 10;
uint32_t options = 0;
- int64_t resource_id = GetNewResourceIdSync(context()->storage());
+ int64_t resource_id = GetNewResourceIdSync(context()->GetStorageControl());
network::ResourceRequest request;
request.url = url;
diff --git a/content/browser/service_worker/service_worker_storage_unittest.cc b/content/browser/service_worker/service_worker_storage_unittest.cc
index 03d96d5..2fe0ac5 100644
--- a/content/browser/service_worker/service_worker_storage_unittest.cc
+++ b/content/browser/service_worker/service_worker_storage_unittest.cc
@@ -1304,8 +1304,8 @@
script_ = GURL("http://www.test.not/script.js");
import_ = GURL("http://www.test.not/import.js");
document_url_ = GURL("http://www.test.not/scope/document.html");
- resource_id1_ = GetNewResourceIdSync(storage());
- resource_id2_ = GetNewResourceIdSync(storage());
+ resource_id1_ = GetNewResourceIdSync(storage_control());
+ resource_id2_ = GetNewResourceIdSync(storage_control());
resource_id1_size_ = 239193;
resource_id2_size_ = 59923;
@@ -1380,7 +1380,7 @@
WriteMetadataWithServiceWorkerResponseMetadataWriter) {
const char kMetadata1[] = "Test metadata";
const char kMetadata2[] = "small";
- int64_t new_resource_id_ = GetNewResourceIdSync(storage());
+ int64_t new_resource_id_ = GetNewResourceIdSync(storage_control());
// Writing metadata to nonexistent resoirce ID must fail.
EXPECT_GE(0, WriteResponseMetadata(storage_control(), new_resource_id_,
kMetadata1));
@@ -1544,7 +1544,7 @@
EXPECT_TRUE(VerifyBasicResponse(storage_control(), resource_id2_, true));
// Also add an uncommitted resource.
- int64_t kStaleUncommittedResourceId = GetNewResourceIdSync(storage());
+ int64_t kStaleUncommittedResourceId = GetNewResourceIdSync(storage_control());
registry()->StoreUncommittedResourceId(kStaleUncommittedResourceId,
registration_->scope());
verify_ids = GetUncommittedResourceIdsFromDB();
@@ -1561,7 +1561,7 @@
// Store a new uncommitted resource. This triggers stale resource cleanup.
base::RunLoop loop;
storage()->SetPurgingCompleteCallbackForTest(loop.QuitClosure());
- int64_t kNewResourceId = GetNewResourceIdSync(storage());
+ int64_t kNewResourceId = GetNewResourceIdSync(storage_control());
WriteBasicResponse(storage_control(), kNewResourceId);
registry()->StoreUncommittedResourceId(kNewResourceId,
registration_->scope());
diff --git a/content/browser/service_worker/service_worker_test_utils.cc b/content/browser/service_worker/service_worker_test_utils.cc
index e2438a5..0563e5de 100644
--- a/content/browser/service_worker/service_worker_test_utils.cc
+++ b/content/browser/service_worker/service_worker_test_utils.cc
@@ -518,15 +518,8 @@
std::move(writer), std::move(callback)));
}
-std::unique_ptr<ServiceWorkerResponseWriter> CreateNewResponseWriterSync(
- ServiceWorkerStorage* storage) {
- base::RunLoop run_loop;
- std::unique_ptr<ServiceWorkerResponseWriter> writer;
- int64_t resource_id = GetNewResourceIdSync(storage);
- return storage->CreateResponseWriter(resource_id);
-}
-
-int64_t GetNewResourceIdSync(ServiceWorkerStorage* storage) {
+int64_t GetNewResourceIdSync(
+ mojo::Remote<storage::mojom::ServiceWorkerStorageControl>& storage) {
base::RunLoop run_loop;
int64_t resource_id;
storage->GetNewResourceId(
diff --git a/content/browser/service_worker/service_worker_test_utils.h b/content/browser/service_worker/service_worker_test_utils.h
index d372341b..fdbc11b 100644
--- a/content/browser/service_worker/service_worker_test_utils.h
+++ b/content/browser/service_worker/service_worker_test_utils.h
@@ -201,13 +201,9 @@
const std::string& meta_data,
WriteToDiskCacheCallback callback);
-// Calls ServiceWorkerStorage::CreateNewResponseWriter() and returns the
-// created writer synchronously.
-std::unique_ptr<ServiceWorkerResponseWriter> CreateNewResponseWriterSync(
- ServiceWorkerStorage* storage);
-
-// Calls ServiceWorkerStorage::GetNewResourceId() synchronously.
-int64_t GetNewResourceIdSync(ServiceWorkerStorage* storage);
+// Calls ServiceWorkerStorageControl::GetNewResourceId() synchronously.
+int64_t GetNewResourceIdSync(
+ mojo::Remote<storage::mojom::ServiceWorkerStorageControl>& storage);
// A test implementation of ServiceWorkerResourceReader.
//
