New issue
Advanced search Search tips

Issue 780366 link

Starred by 2 users
Status: Fixed
Owner:
leon....@intel.com
Closed: Mar 2018
Cc:
falken@chromium.org
kinuko@chromium.org
leon....@intel.com
shimazu@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 782528
Blocking:
issue 772713



Sign in to add a comment

Make all mojom calls pass ServiceWorkerObjectInfoPtr rather than ServiceWorkerObjectInfo

Project Member Reported by leon....@intel.com, Nov 1 2017 
I looked though and found the only one mojom call passing blink::mojom::ServiceWorkerObjectInfo struct(relying on corresponding legacy struct traits) is ServiceWorkerEventDispatcher.DispatchExtendableMessageEvent() at https://cs.chromium.org/chromium/src/content/common/service_worker/service_worker_event_dispatcher.mojom?rcl=003376aa937b99fc6d7e182093ef55f077860945&l=139.

struct ExtendableMessageEvent {
  mojo.common.mojom.String16 message;
  url.mojom.Origin source_origin;
  array<handle<message_pipe>> message_ports;
  ExtendableMessageEventSource source;
};

[Native]
struct ExtendableMessageEventSource; 
   ---> typemapping with content::ExtendableMessageEventSource defined in service_worker_types.h:
struct ExtendableMessageEventSource {
  XXXXXXXXXXXX
  // Exactly one of these infos should be valid.
  ServiceWorkerClientInfo client_info;
  blink::mojom::ServiceWorkerObjectInfo service_worker_info;
};


Suggestion:
Eliminate native/mojom struct ExtendableMessageEventSource, instead, create mojom struct ServiceWorkerClientInfo with typemapping.
[Native]
struct ServiceWorkerClientInfo;   ---> Of course this should also disappear eventually, but for now it's just enough to us to be able to pass ServiceWorkerObjectInfoPtr.

struct ExtendableMessageEvent {
  mojo.common.mojom.String16 message;
  url.mojom.Origin source_origin;
  array<handle<message_pipe>> message_ports;
  // Exactly one of |source_client| and |source_service_worker| should be valid.
  ServiceWorkerClientInfo source_client;
  blink::mojom::ServiceWorkerObjectInfo? source_service_worker;
};


WDYT?

Comment 1 by xiaofeng...@intel.com, Nov 1 2017 

IIUC, I think there are three things to do:
1) Create mojom struct ServiceWorkerClientInfo.
2) Eliminate native struct ExtendableMessageEventSource.
3) As the comments in https://chromium-review.googlesource.com/c/chromium/src/+/742237/2/content/common/service_worker/service_worker_container.mojom#96, we can also use blink.mojom.TransferableMessage to replace |message| and |message_ports|.

And I am not sure but prefer to implement #1 firstly, does it make sense?

Comment 2 by leon....@intel.com, Nov 8 2017

Blockedon: 782528

Comment 3 by leon....@intel.com, Nov 8 2017 

Firstly 782528: create mojom struct ServiceWorkerClientInfo in new mojom file third_party/WebKit/public/platform/modules/serviceworker/service_worker_client.mojom

Then, eliminate ExtendableMessageEventSource, instead:
struct ExtendableMessageEvent {
  mojo.common.mojom.String16 message;
  url.mojom.Origin source_origin;
  array<handle<message_pipe>> message_ports;
  // Exactly one of |source_client| and |source_service_worker| should be valid.
  blink::mojom::ServiceWorkerClientInfo? source_client;
  blink::mojom::ServiceWorkerObjectInfo? source_service_worker;
};
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 9 2018 

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

commit a252f5ca0b792b6c0019c576223c5c870df17024
Author: xzhan96 <xiaofeng.zhang@intel.com>
Date: Tue Jan 09 15:49:10 2018

[ServiceWorker] Eliminate native struct ExtendableMessageEventSource

After both SWClientInfo and SWObjectInfo mojofication done, we don't
need this native struct anymore.
And this is also an incremental step toward Onion Soup: having less
dependence on content types for service worker.

BUG= 780366 

Change-Id: Ic1f39ae6366a8a5b2298fc46b8b17cd610c0deab
Reviewed-on: https://chromium-review.googlesource.com/849698
Commit-Queue: Xiaofeng Zhang <xiaofeng.zhang@intel.com>
Reviewed-by: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528003}
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/common/service_worker/service_worker_event_dispatcher.mojom
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/common/service_worker/service_worker_messages.h
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/common/service_worker/service_worker_types.cc
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/common/service_worker/service_worker_types.h
[modify] https://crrev.com/a252f5ca0b792b6c0019c576223c5c870df17024/content/renderer/service_worker/service_worker_context_client.cc

Comment 5 by xiaofeng...@intel.com, Jan 10 2018

Owner: leon....@intel.com

Comment 6 by leon....@intel.com, Mar 5 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment