New issue
Advanced search Search tips

Issue 908344 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Task

Blocking:
issue 789857


Participants' hotlists:
ServiceWorkerOnionSoup


Sign in to add a comment

Onion soup: Remove ServiceWorkerFetchRequest entirely

Project Member Reported by shimazu@chromium.org, Nov 26

Issue description

Currently we are trying to replace content::ServiceWorkerFetchRequest (used in BackgroundFetch and CacheStorage, not in core service worker code) with mojom::FetchAPIRequest in order to remove codes in content/service_worker/common. However, FetchAPIRequest::headers is base::flat_map<std::string, std::string> and it cannot have custom Compare function though SWFetchRequest has it.

Given that the parameter of mojom::ServiceWorker::DispatchFetchEvent() has been changed from SWFetchRequest to network::ResourceRequest, we may be able to remove SWFetchRequest and mojom::FetchAPIRequest and instead we can use network::ResourseRequest, though it might be heavy lifting.

We need to think of how to move forward.

Context: https://chromium-review.googlesource.com/c/chromium/src/+/1328228/8/content/common/service_worker/service_worker_utils.h#b93
 
Description: Show this description
Summary: Onion soup: Remove ServiceWorkerFetchRequest entirely (was: Onion soup: moving mojom::FetchAPIRequest to blink/)
Sorry I found that the title was totally wrong.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c4627d5b85a8f2bc894c409cebbf85715416a33

commit 4c4627d5b85a8f2bc894c409cebbf85715416a33
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Mon Dec 03 13:51:42 2018

Make FetchAPIRequest::headers use case insensitive Compare

This is a prerequisite step for using FetchAPIRequest instead of
ServiceWorkerFetchRequest. ServiceWorkerFetchRequest has a member |headers| with
CaseInsensitiveCompare, but FetchAPIRequest has a member |headers| with the
default Compare function, so that this will cause behavior changes. This CL is
to change FetchAPIRequest::headers to use CaseInsensitiveCompare.

Bug:  908344 , 789854
Change-Id: I8e1736d5c5ab2d291ff7c126afa0f4025983b84d
Reviewed-on: https://chromium-review.googlesource.com/c/1356463
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613074}
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/content/browser/background_fetch/background_fetch_test_base.cc
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/content/common/service_worker/service_worker_utils.cc
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/content/common/service_worker/service_worker_utils.h
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/content/public/test/test_utils.cc
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/common/BUILD.gn
[add] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/common/fetch/OWNERS
[add] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/common/fetch/fetch_api_request_headers.typemap
[add] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/common/fetch/fetch_api_request_headers_map.h
[add] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/common/fetch/fetch_api_request_headers_mojom_traits.h
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/mojom/fetch/fetch_api_request.mojom
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/public/public_typemaps.gni
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/renderer/core/fetch/request_test.cc
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/renderer/platform/mojo/blink_typemaps.gni
[add] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/renderer/platform/mojo/fetch_api_request_headers.typemap
[add] https://crrev.com/4c4627d5b85a8f2bc894c409cebbf85715416a33/third_party/blink/renderer/platform/mojo/fetch_api_request_headers_mojom_traits.h

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0091f7c3822ffbb75d121267d4142e2d26aab49

commit d0091f7c3822ffbb75d121267d4142e2d26aab49
Author: Richard Li <richard.li@intel.com>
Date: Wed Dec 05 06:23:56 2018

[OnionSoup] Remove content::SWFetchRequest completely

This CL removes content::SWFetchRequest completely.

As a part of work of moving fetch_api_request.mojom, the roadmap is as
follow:

1. Remove content::RequestContextType. Use blink.mojom.RequestContextType
instead. --------- CL:1229704

2. Remove content::ServiceWorkerFetchRequest. Use
blink.mojom.FetchAPIRequest instead.  --------CL:1288084, 1312673, This
CL

3. Remove blink::WebURLRequest::RequestContext. Use
blink.mojom.RequestContextType instead. --------- CL:1242301

4. Remove blink::WebServiceWorkerRequest. Use
blink.mojom.FetchAPIRequest instead. ------CL:1322360

5. Remove blink::WebReferrerPolicy. Use blink::mojom::ReferrerPolicy
instead.  --------- CL:1212345

6. Remove content::Referrer, blink::Referrer Use blink::mojom::Referrer
instead. -------CL:1322360

Once all the above tasks get done, fetch_api_request.mojom can be moved
into third_party/blink/public/mojom/fetch/.

Change-Id: I84152bebd57f2a6eb0b96199f99c764d22d3fa5a
Bug:  908344 
Reviewed-on: https://chromium-review.googlesource.com/c/1328228
Commit-Queue: Richard Li <richard.li@intel.com>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613885}
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_delegate_proxy.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_job_controller.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_job_controller_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_request_info.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_scheduler_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_service_impl.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_service_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_test_base.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/background_fetch_test_base.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/storage/create_metadata_task.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/storage/mark_request_complete_task.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/background_fetch/storage/match_requests_task.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_cache.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_cache.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_cache_entry_handler.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_cache_entry_handler.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_cache_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_dispatcher_host.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_manager.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_manager.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/cache_storage/cache_storage_manager_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/common/BUILD.gn
[delete] https://crrev.com/c087a3d5f88eaa4f40e084f562057fa8ad9228e8/content/common/service_worker/service_worker_type_converter.cc
[delete] https://crrev.com/c087a3d5f88eaa4f40e084f562057fa8ad9228e8/content/common/service_worker/service_worker_type_converter.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/common/service_worker/service_worker_types.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/common/service_worker/service_worker_types.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/common/service_worker/service_worker_utils.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/common/service_worker/service_worker_utils.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/public/test/test_utils.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/public/test/test_utils.h
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/renderer/service_worker/service_worker_context_client_unittest.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/renderer/service_worker/service_worker_type_util.cc
[modify] https://crrev.com/d0091f7c3822ffbb75d121267d4142e2d26aab49/content/renderer/service_worker/service_worker_type_util.h

Blocking: 789857
Status: Fixed (was: Started)

Sign in to add a comment