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

Issue 769236 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 629701



Sign in to add a comment

Impl mojo interface ServiceWorkerRegistrationObject

Project Member Reported by leon....@intel.com, Sep 27 2017

Issue description

Put into third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom:
'
interface ServiceWorkerRegistrationObject {
  SetVersionAttributes(int32 changed_mask,
                       ServiceWorkerVersionAttributes attrs);
  UpdateFound();
};
'

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 28 2017

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

commit b4ade5013657cf9b49e4a321a17b190a909d7fe1
Author: Han Leon <leon.han@intel.com>
Date: Thu Sep 28 06:11:19 2017

[ServiceWorker] Do not set version attributes before creation of WebServiceWorkerRegistrationImpl

WebServiceWorkerRegistrationImpl is not yet created because it's only
created once there is a handle_id, which is created by the
ServiceWorkerRegistrationHandle.
Then, before creation of WebServiceWorkerRegistrationImpl instance in
renderer process, trying to send IPC
ServiceWorkerMsg_SetVersionAttributes to it just does nothing.
This CL removes such unnecessary codes.

BUG= 769236 

Change-Id: Icf7f416b9741c2d527dee85be61939f301c2ff89
Reviewed-on: https://chromium-review.googlesource.com/686734
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#504920}
[modify] https://crrev.com/b4ade5013657cf9b49e4a321a17b190a909d7fe1/content/browser/service_worker/service_worker_registration_handle.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 19 2017

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

commit f8ec9afa5db73ca7735c53fadd12fd5a79bca91e
Author: Han Leon <leon.han@intel.com>
Date: Thu Oct 19 06:31:23 2017

[ServiceWorker] Introduce ServiceWorkerRegistrationObject interface.

The notable thing of this CL is that it tied the lifetime of
content::WebServiceWorkerRegistrationImpl with
content::ServiceWorkerRegistrationHandle in the browser process, so that
they have a real 1:1 correspondence with the key registration handle id.

BUG= 769236 

Change-Id: I2773706a8c506bb0523e0e1960abecd65c4cc077
Reviewed-on: https://chromium-review.googlesource.com/686557
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#510015}
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/browser/service_worker/service_worker_registration_handle.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/browser/service_worker/service_worker_registration_handle.h
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/child/service_worker/service_worker_dispatcher.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/child/service_worker/service_worker_dispatcher_unittest.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/child/service_worker/service_worker_provider_context.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/child/service_worker/web_service_worker_provider_impl.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/child/service_worker/web_service_worker_registration_impl.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/child/service_worker/web_service_worker_registration_impl.h
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/f8ec9afa5db73ca7735c53fadd12fd5a79bca91e/third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25 2017

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

commit a6967776c9682d3c69721778cbf2a4003bcd2983
Author: Han Leon <leon.han@intel.com>
Date: Wed Oct 25 05:50:02 2017

[ServicWorker] Manage lifetime of WebServiceWorkerRegistrationImpl

This CL:
 - for service worker global scope execution context, rather than
   binding on the service worker thread, binds impl of interface
   ServiceWorkerRegistrationObject on the IO thread, because it's a
   channel-associated interface which can be bound only on the main or
   IO thread.
 - manages lifetime of WebServiceWorkerRegistrationImpl correctly,
   destroys the instance after BOTH the following 2 conditions were
   fulfilled:
     1. all references to it have been released by the JavaScript
        execution context it lives on.
     2. the ServiceWorkerRegistrationObject Mojo connection has broken.

Before this CL we just delete the WebServiceWorkerRegistrationImpl
instance once the above condition 2 was triggered, not aware that maybe
some of its references are still held by Blink. This leads to the
situation that Blink references to a destroyed instance which may lead
to crashes. This CL fixes these crashes.

BUG=777198, 769236 

Change-Id: I06a6ace4fee9b7ec888d78b31c6a6bc4cdf09c22
Reviewed-on: https://chromium-review.googlesource.com/732998
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511386}
[modify] https://crrev.com/a6967776c9682d3c69721778cbf2a4003bcd2983/content/renderer/service_worker/web_service_worker_registration_impl.cc
[modify] https://crrev.com/a6967776c9682d3c69721778cbf2a4003bcd2983/content/renderer/service_worker/web_service_worker_registration_impl.h

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 31 2017

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

commit c2bbbfb5f8257e94ea42bb37494f57c856a23537
Author: Han Leon <leon.han@intel.com>
Date: Tue Oct 31 14:13:13 2017

[ServiceWorker] Implement ServiceWorkerRegistrationObject.SetVersionAttributes

BUG= 769236 

Change-Id: Ife84ee28396033eabe5d5b0793d895177fe1cf80
Reviewed-on: https://chromium-review.googlesource.com/732146
Commit-Queue: 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>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512823}
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/browser/service_worker/service_worker_provider_host.h
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/browser/service_worker/service_worker_registration_handle.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/browser/service_worker/service_worker_registration_unittest.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/browser/service_worker/service_worker_test_utils.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/common/service_worker/service_worker_messages.h
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/common/service_worker/service_worker_types.h
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/common/service_worker/service_worker_types.mojom
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/common/service_worker/service_worker_types.typemap
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/renderer/service_worker/service_worker_dispatcher.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/renderer/service_worker/service_worker_dispatcher.h
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/renderer/service_worker/service_worker_message_filter.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/renderer/service_worker/service_worker_message_filter.h
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/renderer/service_worker/web_service_worker_registration_impl.cc
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/content/renderer/service_worker/web_service_worker_registration_impl.h
[modify] https://crrev.com/c2bbbfb5f8257e94ea42bb37494f57c856a23537/third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 1 2017

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

commit fd28cc4c5cdd169fa8e66438d8b3302110deb5c2
Author: Han Leon <leon.han@intel.com>
Date: Wed Nov 01 13:27:45 2017

[ServiceWorker] Check state in WebServiceWorkerRegistrationImpl::SetVersionAttributes

BUG= 769236 

Change-Id: Ifeee539a033b93ccee3e2223e7eb36b266385911
Reviewed-on: https://chromium-review.googlesource.com/748905
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#513115}
[modify] https://crrev.com/fd28cc4c5cdd169fa8e66438d8b3302110deb5c2/content/renderer/service_worker/web_service_worker_registration_impl.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 2 2017

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

commit 00ba142a4c286bd2f4579aa2450a17d7dd172158
Author: Han Leon <leon.han@intel.com>
Date: Thu Nov 02 03:07:41 2017

[ServiceWorker] Implement ServiceWorkerRegistrationObject.UpdateFound

BUG= 769236 

Change-Id: I853b5e651e2596dd2b34fe9ad9fbc6ce677cd2df
Reviewed-on: https://chromium-review.googlesource.com/743342
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513387}
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/browser/service_worker/service_worker_provider_host.h
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/browser/service_worker/service_worker_registration_handle.cc
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/browser/service_worker/service_worker_registration_unittest.cc
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/common/service_worker/service_worker_messages.h
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/renderer/service_worker/service_worker_dispatcher.cc
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/renderer/service_worker/service_worker_dispatcher.h
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/renderer/service_worker/web_service_worker_registration_impl.cc
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/content/renderer/service_worker/web_service_worker_registration_impl.h
[modify] https://crrev.com/00ba142a4c286bd2f4579aa2450a17d7dd172158/third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom

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

Status: Fixed (was: Started)
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 15 2017

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

commit 86029dbfbf1c4927e12c8f8b09de04d706553472
Author: Han Leon <leon.han@intel.com>
Date: Wed Nov 15 03:07:51 2017

[ServiceWorker] Eliminate blink.mojom.ServiceWorkerRegistrationObjectInfo.handle_id

This CL stops passing registration handle id from the browser process to
the renderer process.

Before this CL, the renderer process uses the registration handle id to
differentiate a service worker registration entity against multiple
JavaScript execution contexts for service worker clients or service
worker itself, by managing the mapping between registration handle id
and JavaScript registration object in each thread-specific instance of
ServiceWorkerDispatcher.
However, as the registration handle id is actually an unique identifier
of the (provider_id, registration_id) pair, this CL makes the renderer
process manage the mapping between registration id and JavaScript
registration object in each provider-specific instance of
ServiceWorkerProviderContext for all JavaScript execution contexts.
Thus, we do not need to pass the registration handle id in
blink.mojom.ServiceWorkerRegistrationObjectInfo anymore.

Also, we found that we do not need to manage the mapping for service
worker execution context itself, because
ServiceWorkerGlobalScope#registration is just set once immediately after
the service worker thread started and will never change later. So this
CL only manages the mapping for service worker client contexts.

Follow-up CLs will rename ServiceWorkerRegistrationHandle class to
ServiceWorkerRegistrationObjectHost, and then manage them per
ServiceWorkerProviderHost instead of per ServiceWorkerDispatcherHost.

BUG= 769236 
TEST=content_unittests --gtest_filter=ServiceWorkerProviderContextTest.*

Change-Id: Ic9c68565bcb05d6bc40e9f6380bdfe66ab1577cb
Reviewed-on: https://chromium-review.googlesource.com/748907
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516578}
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/browser/service_worker/service_worker_registration_handle.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_context_client.h
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_dispatcher.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_dispatcher.h
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_dispatcher_unittest.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_provider_context.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_provider_context.h
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/service_worker_provider_context_unittest.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/web_service_worker_provider_impl.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/web_service_worker_registration_impl.cc
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/content/renderer/service_worker/web_service_worker_registration_impl.h
[modify] https://crrev.com/86029dbfbf1c4927e12c8f8b09de04d706553472/third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom

Sign in to add a comment