New issue
Advanced search Search tips

Issue 603817 link

Starred by 1 user
Status: Fixed
Owner:
shimazu@chromium.org
Closed: Apr 2016
Cc:
yhirano@chromium.org
nhiroki@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Failure of SWR.unregister() owned by removed iframe

Project Member Reported by shimazu@chromium.org, Apr 15 2016 
Calling ServiceWorkerRegistration.unregister() fails after removing iframe which has the ServiceWorkerRegistration.

The reason might be that ServiceWorkerRegistration::m_provider is not updated even after removal of iframe, and existence check would fail: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.cpp&sq=package:chromium&type=cs&l=91

According to off-line talk with nhiroki@, the provider should be created by using ServiceWorkerContainerClient::from(getExecutionContext()) every time (ref: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/modules/serviceworkers/ServiceWorker.cpp&sq=package:chromium&type=cs&l=54 )


== Test code to reproduce ==

<!DOCTYPE html>
<meta charset="utf-8">
<title>Service Worker: Reclaiming registration after removal of iframe</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script src="../resources/test-helpers.js"></script>
<script>
promise_test(function(t) {
    var url = 'resources/blank.html';
    var scope = 'resources/reclam-registration-after-iframe-deletion';
    var script = 'resources/empty-worker.js';
    var frame;
    var registration;
   
    return service_worker_unregister(t, scope)
      .then(function() {
          return with_iframe(url, {auto_remove: false});
        })
      .then(function(f) {
          frame = f;
          return frame.contentWindow.navigator.serviceWorker.register(
            script,
            { scope: scope });
        })
      .then(function(r) {
          registration = r;
          frame.remove();
        })
      .then(function() {
          return registration.unregister();
        });
  }, 'unregistration should be finished successfully');
</script>

Comment 1 by shimazu@chromium.org, Apr 18 2016

Status: Started (was: Assigned)
On this test, the last registration.unregister() seems to return 'undefined' instead of 'Promise<boolean>'. 
Expected behavior is to return a rejected promise, I guess.
I'm investigating this problem now.

Comment 2 by nhiroki@chromium.org, Apr 18 2016 

The test code causes a renderer crash due to a bad IPC message (SWDH_UNREGISTER_NO_HOST = 53). This could be a reason why the rate of SWDH_UNREGISTER_NO_HOST is slightly high on Stability.BadMessageTerminated.Content UMA.

Comment 3 by shimazu@chromium.org, Apr 18 2016

Cc: yhirano@chromium.org

Comment 4 by shimazu@chromium.org, Apr 18 2016 

Before executing this test, you have to apply this CL: http://crrev.com/1894033002
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 26 2016 

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

commit e0281ef6c52c10a3baf7e0a043c944ab14c9a712
Author: shimazu <shimazu@chromium.org>
Date: Tue Apr 26 05:02:18 2016

ServiceWorker: Refer to the current provider in ServiceWorkerRegistration

Currently |m_provider| is kept at the constructor in ServiceWorkerRegistration.
Once the provider is removed (e.g. the registration is belong to an iframe and
iframe.remove() is called), the registration would refer the invalid
|m_provider|. This patch removes |m_provider| and gets a corresponding provider
via SWContainerClient.
Additionally, reject is changed to rejectWithDOMException to reject immediately.
This change enables us to get DOMException on the parent frame used to own the
reclaimed iframe. A test is also added for tracking the exception.

BUG= 603817 
TEST=./third_party/WebKit/Tools/Scripts/run-webkit-tests -t Default http/tests/serviceworker/chromium/unregister-on-detached-iframe.html

Review URL: https://codereview.chromium.org/1893363003

Cr-Commit-Position: refs/heads/master@{#389700}

[add] https://crrev.com/e0281ef6c52c10a3baf7e0a043c944ab14c9a712/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/unregister-on-detached-iframe.html
[modify] https://crrev.com/e0281ef6c52c10a3baf7e0a043c944ab14c9a712/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.cpp
[modify] https://crrev.com/e0281ef6c52c10a3baf7e0a043c944ab14c9a712/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.h

Comment 6 by shimazu@chromium.org, Apr 26 2016

Status: Fixed (was: Started)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 9 2017 

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

commit 194e15e332e1af79dfe4813d1e0c1c36d1f17d95
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Nov 09 02:44:42 2017

service worker: Expand and upstream tests on detached contexts.

This upstreams service worker tests:
chromium/extendable-message-event.html
chromium/unregister-on-detached-iframe.html

These tests are useful as a WPT as it tests web-exposed behavior.

Expand the tests and upstream them as detached-context.https.html.

Bug:  688116 ,  603817 ,  543198 
Change-Id: I2b9798e19a1b1ec76d9a4472f412acb324a50e0c
Reviewed-on: https://chromium-review.googlesource.com/759136
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515070}
[add] https://crrev.com/194e15e332e1af79dfe4813d1e0c1c36d1f17d95/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/detached-context.https.html
[delete] https://crrev.com/a9de482718f2f065ebd47963ca3d82485a7a4833/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/extendable-message-event.html
[delete] https://crrev.com/a9de482718f2f065ebd47963ca3d82485a7a4833/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/unregister-on-detached-iframe.html
►

Sign in to add a comment