Commit fcd63c07 authored by Han Leon's avatar Han Leon Committed by Commit Bot

[OnionSoup] Remove [Native] content.mojom.ServiceWorkerResponse

This CL is the 4th step of the plan below aiming to eliminate the native
struct content::ServiceWorkerResponse defined in
content/common/service_worker/service_worker_types.h.

1st step:
https://chromium-review.googlesource.com/c/chromium/src/+/1134731

2nd step:
https://chromium-review.googlesource.com/c/chromium/src/+/1149706

3rd step:
https://chromium-review.googlesource.com/c/chromium/src/+/1155999

4th step: (this CL)
- Use blink.mojom.FetchAPIResponse to represent responses to background
  fetches. Specifically, use this mojom struct to replace the existing
  [Native] mojom struct content.mojom.ServiceWorkerResponse defined in
  content/common/service_worker/service_worker.mojom.
  Thus we can reduce the last 1 user of content::ServiceWorkerResponse.

5th step:
- Remove content::ServiceWorkerResponse and do any left cleanup.

BUG=789854

Change-Id: I85e6e9489dac50e1cc6905bd28a2d47b7a90fde7
Reviewed-on: https://chromium-review.googlesource.com/1164848Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582489}
parent 3141871b
......@@ -1347,9 +1347,9 @@ TEST_F(BackgroundFetchDataManagerTest, GetSettledFetchesFromCache) {
// Sanity check that the responses are written to / read from the cache.
EXPECT_TRUE(MatchCache(requests[0]));
EXPECT_TRUE(MatchCache(requests[1]));
EXPECT_EQ(settled_fetches[0].response.cache_storage_cache_name,
EXPECT_EQ(settled_fetches[0].response->cache_storage_cache_name,
kExampleUniqueId);
EXPECT_EQ(settled_fetches[1].response.cache_storage_cache_name,
EXPECT_EQ(settled_fetches[1].response->cache_storage_cache_name,
kExampleUniqueId);
RestartDataManagerFromPersistentStorage();
......@@ -1444,7 +1444,7 @@ TEST_F(BackgroundFetchDataManagerTest,
&succeeded, &settled_fetches);
ASSERT_EQ(error, blink::mojom::BackgroundFetchError::NONE);
EXPECT_EQ(settled_fetches.size(), 1u);
EXPECT_EQ(settled_fetches[0].response.response_type,
EXPECT_EQ(settled_fetches[0].response->response_type,
network::mojom::FetchResponseType::kError);
}
......
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <algorithm>
#include <map>
#include <memory>
#include <utility>
......@@ -9,6 +10,7 @@
#include "base/auto_reset.h"
#include "base/macros.h"
#include "base/run_loop.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/time/time.h"
......@@ -47,6 +49,16 @@ blink::Manifest::ImageResource CreateIcon(const std::string& src,
return icon;
}
bool ContainsHeader(const base::flat_map<std::string, std::string>& headers,
const std::string& target) {
return headers.cend() !=
std::find_if(headers.cbegin(), headers.cend(),
[target](const auto& pair) -> bool {
return base::CompareCaseInsensitiveASCII(pair.first,
target) == 0;
});
}
class BadMessageObserver {
public:
BadMessageObserver()
......@@ -529,41 +541,45 @@ TEST_F(BackgroundFetchServiceTest, FetchSuccessEventDispatch) {
ASSERT_EQ(fetches[i].request.url, requests[i].url);
EXPECT_EQ(fetches[i].request.method, requests[i].method);
EXPECT_EQ(fetches[i].response.url_list[0], fetches[i].request.url);
EXPECT_EQ(fetches[i].response.response_type,
EXPECT_EQ(fetches[i].response->url_list[0], fetches[i].request.url);
EXPECT_EQ(fetches[i].response->response_type,
network::mojom::FetchResponseType::kDefault);
switch (i) {
case 0:
EXPECT_EQ(fetches[i].response.status_code, kFirstResponseCode);
EXPECT_EQ(fetches[i].response.headers.count("Content-Type"), 1u);
EXPECT_EQ(fetches[i].response.headers.count("X-Cat"), 1u);
EXPECT_EQ(fetches[i].response->status_code, kFirstResponseCode);
EXPECT_TRUE(
ContainsHeader(fetches[i].response->headers, "Content-Type"));
EXPECT_TRUE(ContainsHeader(fetches[i].response->headers, "X-Cat"));
break;
case 1:
EXPECT_EQ(fetches[i].response.status_code, kSecondResponseCode);
EXPECT_EQ(fetches[i].response.headers.count("Content-Type"), 1u);
EXPECT_EQ(fetches[i].response.headers.count("X-Cat"), 0u);
EXPECT_EQ(fetches[i].response->status_code, kSecondResponseCode);
EXPECT_TRUE(
ContainsHeader(fetches[i].response->headers, "Content-Type"));
EXPECT_FALSE(ContainsHeader(fetches[i].response->headers, "X-Cat"));
break;
case 2:
EXPECT_EQ(fetches[i].response.status_code, kThirdResponseCode);
EXPECT_EQ(fetches[i].response.headers.count("Content-Type"), 1u);
EXPECT_EQ(fetches[i].response.headers.count("X-Cat"), 0u);
EXPECT_EQ(fetches[i].response->status_code, kThirdResponseCode);
EXPECT_TRUE(
ContainsHeader(fetches[i].response->headers, "Content-Type"));
EXPECT_FALSE(ContainsHeader(fetches[i].response->headers, "X-Cat"));
break;
default:
NOTREACHED();
}
// TODO(peter): change-detector tests for unsupported properties.
EXPECT_EQ(fetches[i].response.error,
EXPECT_EQ(fetches[i].response->error,
blink::mojom::ServiceWorkerResponseError::kUnknown);
// Verify that all properties have a sensible value.
EXPECT_FALSE(fetches[i].response.response_time.is_null());
EXPECT_FALSE(fetches[i].response->response_time.is_null());
// Verify that the response blobs have been populated. We cannot consume
// their data here since the handles have already been released.
ASSERT_FALSE(fetches[i].response.blob_uuid.empty());
ASSERT_GT(fetches[i].response.blob_size, 0u);
ASSERT_TRUE(fetches[i].response->blob);
ASSERT_FALSE(fetches[i].response->blob->uuid.empty());
ASSERT_GT(fetches[i].response->blob->size, 0u);
}
}
......@@ -628,30 +644,29 @@ TEST_F(BackgroundFetchServiceTest, FetchFailEventDispatch) {
ASSERT_EQ(fetches[i].request.url, requests[i].url);
EXPECT_EQ(fetches[i].request.method, requests[i].method);
EXPECT_EQ(fetches[i].response.url_list[0], fetches[i].request.url);
EXPECT_EQ(fetches[i].response.response_type,
EXPECT_EQ(fetches[i].response->url_list[0], fetches[i].request.url);
EXPECT_EQ(fetches[i].response->response_type,
network::mojom::FetchResponseType::kDefault);
switch (i) {
case 0:
EXPECT_EQ(fetches[i].response.status_code, 404);
EXPECT_EQ(fetches[i].response->status_code, 404);
break;
case 1:
EXPECT_EQ(fetches[i].response.status_code, 0);
EXPECT_EQ(fetches[i].response->status_code, 0);
break;
default:
NOTREACHED();
}
EXPECT_TRUE(fetches[i].response.headers.empty());
EXPECT_TRUE(fetches[i].response.blob_uuid.empty());
EXPECT_EQ(fetches[i].response.blob_size, 0u);
EXPECT_FALSE(fetches[i].response.response_time.is_null());
EXPECT_TRUE(fetches[i].response->headers.empty());
EXPECT_FALSE(fetches[i].response->blob);
EXPECT_FALSE(fetches[i].response->response_time.is_null());
// TODO(peter): change-detector tests for unsupported properties.
EXPECT_EQ(fetches[i].response.error,
EXPECT_EQ(fetches[i].response->error,
blink::mojom::ServiceWorkerResponseError::kUnknown);
EXPECT_TRUE(fetches[i].response.cors_exposed_header_names.empty());
EXPECT_TRUE(fetches[i].response->cors_exposed_header_names.empty());
}
}
......
......@@ -10,49 +10,6 @@
#include "content/browser/cache_storage/cache_storage_manager.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h"
namespace mojo {
// TODO(leonhsl): Deprecate this converter once we replace usages of
// content::ServiceWorkerResponse with blink::mojom::FetchAPIResponse in
// background_fetch codes.
template <>
struct TypeConverter<content::ServiceWorkerResponse,
blink::mojom::FetchAPIResponse> {
static content::ServiceWorkerResponse Convert(
const blink::mojom::FetchAPIResponse& input) {
content::ServiceWorkerResponse output;
output.url_list = std::move(input.url_list);
output.status_code = input.status_code;
output.status_text = std::move(input.status_text);
output.response_type = input.response_type;
output.headers.insert(input.headers.begin(), input.headers.end());
if (input.blob) {
output.blob_uuid = input.blob->uuid;
output.blob_size = input.blob->size;
output.blob = base::MakeRefCounted<storage::BlobHandle>(
blink::mojom::BlobPtr(std::move(input.blob->blob)));
}
output.error = input.error;
output.response_time = input.response_time;
output.is_in_cache_storage = input.is_in_cache_storage;
if (input.cache_storage_cache_name) {
output.cache_storage_cache_name =
std::move(*(input.cache_storage_cache_name));
}
output.cors_exposed_header_names =
std::move(input.cors_exposed_header_names);
if (input.side_data_blob) {
output.side_data_blob_uuid = input.side_data_blob->uuid;
output.side_data_blob_size = input.side_data_blob->size;
output.side_data_blob = base::MakeRefCounted<storage::BlobHandle>(
blink::mojom::BlobPtr(std::move(input.side_data_blob->blob)));
}
return output;
}
};
} // namespace mojo
namespace content {
namespace background_fetch {
......@@ -214,7 +171,7 @@ void GetSettledFetchesTask::DidMatchRequest(
FillUncachedResponse(settled_fetch, std::move(callback));
return;
}
settled_fetch->response = cache_response->To<ServiceWorkerResponse>();
settled_fetch->response = std::move(cache_response);
std::move(callback).Run();
}
......@@ -224,9 +181,11 @@ void GetSettledFetchesTask::FillUncachedResponse(
background_fetch_succeeded_ = false;
// TODO(rayankans): Fill unmatched response with error reports.
settled_fetch->response.response_type =
DCHECK(!settled_fetch->response);
settled_fetch->response = blink::mojom::FetchAPIResponse::New();
settled_fetch->response->response_type =
network::mojom::FetchResponseType::kError;
settled_fetch->response.url_list.push_back(settled_fetch->request.url);
settled_fetch->response->url_list.push_back(settled_fetch->request.url);
std::move(callback).Run();
}
......
......@@ -83,9 +83,10 @@ struct CONTENT_EXPORT
const content::BackgroundFetchSettledFetch& fetch) {
return fetch.request;
}
static const content::ServiceWorkerResponse& response(
static blink::mojom::FetchAPIResponsePtr response(
const content::BackgroundFetchSettledFetch& fetch) {
return fetch.response;
return content::BackgroundFetchSettledFetch::MakeCloneResponse(
fetch.response);
}
static bool Read(content::mojom::BackgroundFetchSettledFetchDataView data,
......
......@@ -4,6 +4,20 @@
#include "content/common/background_fetch/background_fetch_types.h"
namespace {
blink::mojom::SerializedBlobPtr MakeCloneSerializedBlob(
const blink::mojom::SerializedBlobPtr& blob) {
if (!blob)
return nullptr;
blink::mojom::BlobPtr blob_ptr(std::move(blob->blob));
blob_ptr->Clone(mojo::MakeRequest(&blob->blob));
return blink::mojom::SerializedBlob::New(
blob->uuid, blob->content_type, blob->size, blob_ptr.PassInterface());
}
} // namespace
namespace content {
BackgroundFetchOptions::BackgroundFetchOptions() = default;
......@@ -20,10 +34,33 @@ BackgroundFetchRegistration::BackgroundFetchRegistration(
BackgroundFetchRegistration::~BackgroundFetchRegistration() = default;
// static
blink::mojom::FetchAPIResponsePtr
BackgroundFetchSettledFetch::MakeCloneResponse(
const blink::mojom::FetchAPIResponsePtr& response) {
if (!response)
return nullptr;
return blink::mojom::FetchAPIResponse::New(
response->url_list, response->status_code, response->status_text,
response->response_type, response->headers,
MakeCloneSerializedBlob(response->blob), response->error,
response->response_time, response->cache_storage_cache_name,
response->cors_exposed_header_names, response->is_in_cache_storage,
MakeCloneSerializedBlob(response->side_data_blob));
}
BackgroundFetchSettledFetch::BackgroundFetchSettledFetch() = default;
BackgroundFetchSettledFetch::BackgroundFetchSettledFetch(
const BackgroundFetchSettledFetch& other) = default;
const BackgroundFetchSettledFetch& other) {
*this = other;
}
BackgroundFetchSettledFetch& BackgroundFetchSettledFetch::operator=(
const BackgroundFetchSettledFetch& other) {
request = other.request;
response = MakeCloneResponse(other.response);
return *this;
}
BackgroundFetchSettledFetch::~BackgroundFetchSettledFetch() = default;
......
......@@ -12,6 +12,7 @@
#include "content/common/content_export.h"
#include "content/common/service_worker/service_worker_types.h"
#include "third_party/blink/public/common/manifest/manifest.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_response.mojom.h"
namespace content {
......@@ -55,12 +56,16 @@ struct CONTENT_EXPORT BackgroundFetchRegistration {
// Analogous to the following structure in the spec:
// http://wicg.github.io/background-fetch/#backgroundfetchsettledfetch
struct CONTENT_EXPORT BackgroundFetchSettledFetch {
static blink::mojom::FetchAPIResponsePtr MakeCloneResponse(
const blink::mojom::FetchAPIResponsePtr& response);
BackgroundFetchSettledFetch();
BackgroundFetchSettledFetch(const BackgroundFetchSettledFetch& other);
BackgroundFetchSettledFetch& operator=(
const BackgroundFetchSettledFetch& other);
~BackgroundFetchSettledFetch();
ServiceWorkerFetchRequest request;
ServiceWorkerResponse response;
blink::mojom::FetchAPIResponsePtr response;
};
} // namespace content
......
......@@ -10,6 +10,7 @@ import "services/network/public/mojom/cookie_manager.mojom";
import "third_party/blink/public/platform/modules/fetch/fetch_api_request.mojom";
import "third_party/blink/public/platform/modules/notifications/notification.mojom";
import "third_party/blink/public/platform/modules/payments/payment_app.mojom";
import "third_party/blink/public/mojom/fetch/fetch_api_response.mojom";
import "third_party/blink/public/mojom/message_port/message_port.mojom";
import "third_party/blink/public/mojom/service_worker/dispatch_fetch_event_params.mojom";
import "third_party/blink/public/mojom/service_worker/service_worker.mojom";
......@@ -21,14 +22,10 @@ import "third_party/blink/public/mojom/service_worker/service_worker_registratio
import "url/mojom/origin.mojom";
import "url/mojom/url.mojom";
[Native]
struct ServiceWorkerResponse;
// TODO(peter): Move this to Blink when ServiceWorkerResponse has a Mojo
// counterpart.
// TODO(peter): Move this to Blink.
struct BackgroundFetchSettledFetch {
blink.mojom.FetchAPIRequest request;
ServiceWorkerResponse response;
blink.mojom.FetchAPIResponse? response;
};
enum BackgroundFetchState {
......
# Copyright 2016 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.
mojom = "//content/common/service_worker/service_worker.mojom"
public_headers = [ "//content/common/service_worker/service_worker_types.h" ]
traits_headers = [ "//content/common/service_worker/service_worker_messages.h" ]
type_mappings = [
"content.mojom.ServiceWorkerFetchRequest=::content::ServiceWorkerFetchRequest",
"content.mojom.ServiceWorkerResponse=::content::ServiceWorkerResponse",
]
......@@ -27,9 +27,6 @@
// TODO(leonhsl): Figure out what's the purpose of all these traits then
// eliminate this file finally.
IPC_ENUM_TRAITS_MAX_VALUE(blink::mojom::ServiceWorkerResponseError,
blink::mojom::ServiceWorkerResponseError::kMaxValue)
IPC_STRUCT_TRAITS_BEGIN(content::ServiceWorkerFetchRequest)
IPC_STRUCT_TRAITS_MEMBER(mode)
IPC_STRUCT_TRAITS_MEMBER(is_main_resource_load)
......@@ -47,23 +44,4 @@ IPC_STRUCT_TRAITS_BEGIN(content::ServiceWorkerFetchRequest)
IPC_STRUCT_TRAITS_MEMBER(is_reload)
IPC_STRUCT_TRAITS_END()
IPC_STRUCT_TRAITS_BEGIN(content::ServiceWorkerResponse)
IPC_STRUCT_TRAITS_MEMBER(url_list)
IPC_STRUCT_TRAITS_MEMBER(status_code)
IPC_STRUCT_TRAITS_MEMBER(status_text)
IPC_STRUCT_TRAITS_MEMBER(response_type)
IPC_STRUCT_TRAITS_MEMBER(headers)
IPC_STRUCT_TRAITS_MEMBER(blob_uuid)
IPC_STRUCT_TRAITS_MEMBER(blob_size)
IPC_STRUCT_TRAITS_MEMBER(blob)
IPC_STRUCT_TRAITS_MEMBER(error)
IPC_STRUCT_TRAITS_MEMBER(response_time)
IPC_STRUCT_TRAITS_MEMBER(is_in_cache_storage)
IPC_STRUCT_TRAITS_MEMBER(cache_storage_cache_name)
IPC_STRUCT_TRAITS_MEMBER(cors_exposed_header_names)
IPC_STRUCT_TRAITS_MEMBER(side_data_blob_uuid)
IPC_STRUCT_TRAITS_MEMBER(side_data_blob_size)
IPC_STRUCT_TRAITS_MEMBER(side_data_blob)
IPC_STRUCT_TRAITS_END()
#endif // CONTENT_COMMON_SERVICE_WORKER_SERVICE_WORKER_MESSAGES_H_
......@@ -15,7 +15,6 @@ typemaps = [
"//content/common/notifications/notification_types.typemap",
"//content/common/push_messaging.typemap",
"//content/common/render_frame_metadata.typemap",
"//content/common/service_worker/service_worker.typemap",
"//content/common/service_worker/service_worker_fetch_request.typemap",
"//content/common/url_loader_factory_bundle.typemap",
"//content/common/web_preferences.typemap",
......
......@@ -282,40 +282,43 @@ void ToWebServiceWorkerRequest(const ServiceWorkerFetchRequest& request,
}
// Converts |response| to its equivalent type in the Blink API.
// TODO(peter): Remove this when the Mojo FetchAPIResponse type exists.
void ToWebServiceWorkerResponse(const ServiceWorkerResponse& response,
// TODO(leonhsl): Remove this when we propagate
// blink::mojom::FetchAPIResponsePtr into Blink instead of
// WebServiceWorkerResponse.
void ToWebServiceWorkerResponse(blink::mojom::FetchAPIResponse* response,
blink::WebServiceWorkerResponse* web_response) {
DCHECK(web_response);
std::vector<blink::WebURL> url_list;
for (const GURL& url : response.url_list)
for (const GURL& url : response->url_list)
url_list.push_back(blink::WebURL(url));
web_response->SetURLList(blink::WebVector<blink::WebURL>(url_list));
web_response->SetStatus(static_cast<unsigned short>(response.status_code));
web_response->SetStatusText(blink::WebString::FromUTF8(response.status_text));
web_response->SetResponseType(response.response_type);
for (const auto& pair : response.headers) {
web_response->SetStatus(static_cast<unsigned short>(response->status_code));
web_response->SetStatusText(
blink::WebString::FromUTF8(response->status_text));
web_response->SetResponseType(response->response_type);
for (const auto& pair : response->headers) {
web_response->SetHeader(blink::WebString::FromUTF8(pair.first),
blink::WebString::FromUTF8(pair.second));
}
if (!response.blob_uuid.empty()) {
DCHECK(response.blob);
mojo::ScopedMessagePipeHandle blob_pipe;
if (response.blob)
blob_pipe = response.blob->Clone().PassInterface().PassHandle();
web_response->SetBlob(blink::WebString::FromASCII(response.blob_uuid),
response.blob_size, std::move(blob_pipe));
if (response->blob) {
DCHECK(!response->blob->uuid.empty());
DCHECK(response->blob->blob.is_valid());
web_response->SetBlob(blink::WebString::FromASCII(response->blob->uuid),
response->blob->size,
response->blob->blob.PassHandle());
}
web_response->SetError(response.error);
web_response->SetResponseTime(response.response_time);
if (response.is_in_cache_storage) {
web_response->SetCacheStorageCacheName(
blink::WebString::FromUTF8(response.cache_storage_cache_name));
web_response->SetError(response->error);
web_response->SetResponseTime(response->response_time);
if (response->is_in_cache_storage) {
web_response->SetCacheStorageCacheName(blink::WebString::FromUTF8(
response->cache_storage_cache_name ? *response->cache_storage_cache_name
: ""));
}
std::vector<blink::WebString> cors_exposed_header_names;
for (const auto& name : response.cors_exposed_header_names)
for (const auto& name : response->cors_exposed_header_names)
cors_exposed_header_names.push_back(blink::WebString::FromUTF8(name));
web_response->SetCorsExposedHeaderNames(
......@@ -1595,7 +1598,8 @@ void ServiceWorkerContextClient::DispatchBackgroundFetchAbortEvent(
fetches.size());
for (size_t i = 0; i < fetches.size(); ++i) {
ToWebServiceWorkerRequest(fetches[i].request, &web_fetches[i].request);
ToWebServiceWorkerResponse(fetches[i].response, &web_fetches[i].response);
ToWebServiceWorkerResponse(fetches[i].response.get(),
&web_fetches[i].response);
}
proxy_->DispatchBackgroundFetchAbortEvent(
......@@ -1646,7 +1650,8 @@ void ServiceWorkerContextClient::DispatchBackgroundFetchFailEvent(
fetches.size());
for (size_t i = 0; i < fetches.size(); ++i) {
ToWebServiceWorkerRequest(fetches[i].request, &web_fetches[i].request);
ToWebServiceWorkerResponse(fetches[i].response, &web_fetches[i].response);
ToWebServiceWorkerResponse(fetches[i].response.get(),
&web_fetches[i].response);
}
proxy_->DispatchBackgroundFetchFailEvent(
......@@ -1674,7 +1679,8 @@ void ServiceWorkerContextClient::DispatchBackgroundFetchedEvent(
fetches.size());
for (size_t i = 0; i < fetches.size(); ++i) {
ToWebServiceWorkerRequest(fetches[i].request, &web_fetches[i].request);
ToWebServiceWorkerResponse(fetches[i].response, &web_fetches[i].response);
ToWebServiceWorkerResponse(fetches[i].response.get(),
&web_fetches[i].response);
}
proxy_->DispatchBackgroundFetchedEvent(
......
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