IDL binding + test code for .current (no support for currentchanged event yet). Test code ...
6 years, 7 months ago
(2014-05-02 12:26:12 UTC)
#1
IDL binding + test code for .current (no support for currentchanged event yet).
Test code is mostly just copied from Matt's github code.
PTL, and one question for binding/js gurus
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/current-on-reload.html (right):
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/current-on-reload.html:38: //
assert_true(w.navigator.serviceWorker.current instanceof ServiceWorker,
Somehow I can't make it work for .current. I'm likely missing something? Can
binding/js gurus help me out?
6 years, 7 months ago
(2014-05-02 17:55:02 UTC)
#2
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/current-on-reload.html (right):
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/current-on-reload.html:38: //
assert_true(w.navigator.serviceWorker.current instanceof ServiceWorker,
On 2014/05/02 12:26:13, kinuko wrote:
> Somehow I can't make it work for .current. I'm likely missing something? Can
> binding/js gurus help me out?
w.navigator.serviceWorker.current is reaching into another window to dig out the
object. Objects from another window are in a different "realm" - they have
different global and intrinsics. (That's why most real-world JS eschews
instanceof and uses things like Array.isArray() instead.)
This should work:
assert_true(w.navigator.serviceWorker.current instanceof w.ServiceWorker);
I'll give it a try when my build is done.
jsbell
> This should work: > > assert_true(w.navigator.serviceWorker.current instanceof w.ServiceWorker); > > I'll give it a ...
6 years, 7 months ago
(2014-05-02 19:36:29 UTC)
#3
> This should work:
>
> assert_true(w.navigator.serviceWorker.current instanceof w.ServiceWorker);
>
> I'll give it a try when my build is done.
Yeah, that was it. Works like a charm.
I've "forked" this CL over to: https://codereview.chromium.org/263903005
... which also adds the expectation files and makes a few other tweaks I made
while trying to track down why it was failing for me, until I found
the chromium-side change crrev.com/261533003 :)
Consider this a rietveld "Pull Request" ?
jsbell
lgtm otherwise with the instanceof fix, and if you want to merge any of my ...
6 years, 7 months ago
(2014-05-02 19:42:08 UTC)
#4
Thanks, merged your changes and addressed comments! (Will land once the other patch becomes ready..) ...
6 years, 7 months ago
(2014-05-05 14:20:26 UTC)
#5
Thanks, merged your changes and addressed comments! (Will land once the other
patch becomes ready..)
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/current-on-reload.html (right):
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/current-on-reload.html:38: //
assert_true(w.navigator.serviceWorker.current instanceof ServiceWorker,
On 2014/05/02 17:55:03, jsbell wrote:
> On 2014/05/02 12:26:13, kinuko wrote:
> > Somehow I can't make it work for .current. I'm likely missing something? Can
> > binding/js gurus help me out?
>
> w.navigator.serviceWorker.current is reaching into another window to dig out
the
> object. Objects from another window are in a different "realm" - they have
> different global and intrinsics. (That's why most real-world JS eschews
> instanceof and uses things like Array.isArray() instead.)
>
> This should work:
>
> assert_true(w.navigator.serviceWorker.current instanceof w.ServiceWorker);
>
> I'll give it a try when my build is done.
Very cool, thanks!
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/blank.html (right):
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/blank.html:1: <!DOCTYPE html>
On 2014/05/02 19:42:08, jsbell wrote:
> nit: Technically, it also needs a <title> to be a valid doc.
Done.
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
File LayoutTests/http/tests/serviceworker/resources/testutils.js (right):
https://codereview.chromium.org/265943003/diff/40001/LayoutTests/http/tests/s...
LayoutTests/http/tests/serviceworker/resources/testutils.js:1: function
withIframe(url, f) {
On 2014/05/02 19:42:08, jsbell wrote:
> Add to test-helpers.js instead?
Done. (Merged your change)
falken
cool! lgtm https://codereview.chromium.org/265943003/diff/80001/LayoutTests/http/tests/serviceworker/resources/testutils.js File LayoutTests/http/tests/serviceworker/resources/testutils.js (right): https://codereview.chromium.org/265943003/diff/80001/LayoutTests/http/tests/serviceworker/resources/testutils.js#newcode1 LayoutTests/http/tests/serviceworker/resources/testutils.js:1: function withIframe(url, f) { Can you add ...
6 years, 7 months ago
(2014-05-06 00:50:46 UTC)
#6
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6764)
6 years, 7 months ago
(2014-05-09 02:05:51 UTC)
#12
I needed to rebase LayoutTests/fast/dom/Window/property-access-on-cached-properties-* tests for window.cached_navigator_serviceWorker.current tests. Updated the patch to include those ...
6 years, 7 months ago
(2014-05-09 02:28:11 UTC)
#15
I needed to rebase
LayoutTests/fast/dom/Window/property-access-on-cached-properties-* tests for
window.cached_navigator_serviceWorker.current tests. Updated the patch to
include those rebaselines.
kinuko
The CQ bit was checked by kinuko@chromium.org
6 years, 7 months ago
(2014-05-09 02:28:18 UTC)
#16
Issue 265943003: Add blink-side binding code for navigator.serviceWorker.current
(Closed)
Created 6 years, 7 months ago by kinuko
Modified 6 years, 7 months ago
Reviewers: falken, jsbell, dominicc (has gone to gerrit)
Base URL: svn://svn.chromium.org/blink/trunk
Comments: 10
Chromium Code Reviews
Issue
265943003:
Add blink-side binding code for navigator.serviceWorker.current (Closed)
