New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 755523 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-Servicification



Sign in to add a comment

Make ServiceWorkerFetchResponseCallback work with ServiceWorkerResponse.blob

Project Member Reported by kinuko@chromium.org, Aug 15 2017

Issue description

Making the fetch code work with BlobHandle (held as |blob| in ServiceWorkerResponse in fetch handling case) is critical to make Blob lifetime management work saner esp. when we can't easily rely on implicit IPC ordering assumptions with Browser process (e.g. in Servicification context).

Currently we pass ServiceWorkerResponse via traditional IPC if (and only if) the response has a valid blob uuid (and blob ptr is non-null if kMojoBlobs is enabled), but this code currently doesn't work if we try to pass the response via ServiceWorkerFetchResponseCallback.

Currently it hits serialization / deserialization error in NativeStructSerializer:

https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/lib/native_struct_serialization.h

It looks ParamTraits::GetSize and ::Write for BlobHandle result in different size (attachment may not be supported in native struct in mojo?):

https://cs.chromium.org/chromium/src/content/common/content_param_traits.cc?sq=package:chromium&dr&q=BlobHandle&l=115

ServiceWorkerResponse.blob and storage::BlobHandle are introduced by this change by mek@:
https://chromium-review.googlesource.com/c/567549

 

Comment 1 by yzshen@chromium.org, Aug 15 2017

Recently Mojo serialization has been changed to "grow-buffer-if-necessary". Therefore, we don't need to check ParamTraits::GetSize matches ::Write size any more. That check could be removed.

However, as you pointed out, attachment is not supported when serializing a native struct in Mojo. Extending support is possible, but IMO it is better to use normal mojom typemapping (i.e., define a mojom struct, define corresponding StructTraits specialization.)

Comment 2 by kinuko@chromium.org, Aug 17 2017

Thanks Yuzhu for the context, it makes sense. I agree that we should add mojo typemapping rather than putting energy on making the native struct support in Mojo better :)

Comment 3 by kinuko@chromium.org, Sep 29 2017

Owner: kinuko@chromium.org
Status: Fixed (was: Available)
I think some of my change have fixed this, so let me close!

Comment 4 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment