New issue
Advanced search Search tips

Issue 861564 link

Starred by 3 users
Status: Started
Owner:
hirosh...@chromium.org
Cc:
kouhei@chromium.org
nhiroki@chromium.org
neis@chromium.org
hirosh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 878274
issue 861965
issue 866666
Blocking:
issue 553535
issue 680046
issue 782981
issue 850662


Participants' hotlists:
Worker-OffTheMainThread


Sign in to add a comment

Worker global scope's URL should be response URL (not request URL)

Project Member Reported by hirosh...@chromium.org, Jul 8 
An umbrella issue to replace request URL used as "worker's URL"-ish things with response URL.

Design doc: https://docs.google.com/document/d/1lGBoqIfcQJk1z1ffv-v7GVFJdYw78tXUCRKcAba6nyg/edit?usp=sharing

Comment 1 by hirosh...@chromium.org, Jul 8

Blocking: 782981 850662

Comment 2 by hirosh...@chromium.org, Jul 9

Blockedon: 861965
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 10 

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

commit 739259e76c7efbacdd366642135e2c9c337dc470
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue Jul 10 19:00:46 2018

Add tests for (module dedicated|classic shared) worker's URLs

This CL adds WPT tests for
- WorkerLocation (wpt/workers/interfaces/WorkerGlobalScope/location/),
- referrer (wpt/xhr/), and
- API base URL (wpt/workers/baseurl/)
that should reflect worker global scope's URL, for
- classic shared workers and
- module classic workers.

Some tests are half-migrated to fetch_tests_from_worker()-style
to test shared workers easily, while the tests for
module workers uses the existing non-fetch_tests_from_worker()-style
tests, because testharness.js can't be used from module scripts.

Bug: 861564
Change-Id: I62fa415647da47e00bf7cbf4d7648f5d64377c04
Reviewed-on: https://chromium-review.googlesource.com/1128517
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573840}
[modify] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/TestExpectations
[copy] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/import-in-moduleworker.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/importScripts-in-sharedworker.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/importScripts-in-worker.html
[delete] https://crrev.com/3d6e4b23897a1e7a5ff37838744e8bfe39ff2952/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/importScripts.html
[rename] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/sharedworker-in-worker.html
[rename] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/worker-in-worker.html
[rename] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/xhr-in-moduleworker.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/xhr-in-sharedworker.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/alpha/xhr-in-worker.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/beta/import.py
[modify] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/beta/script.js
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/beta/xhr-worker.py
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/gamma/import.js
[modify] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/gamma/importScripts.js
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/gamma/script-module.js
[modify] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/gamma/script.js
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/baseurl/gamma/xhr-worker.js
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/interfaces/WorkerGlobalScope/location/redirect-module.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/interfaces/WorkerGlobalScope/location/redirect-sharedworker.html
[modify] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/interfaces/WorkerGlobalScope/location/redirect.html
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/workers/interfaces/WorkerGlobalScope/location/redirect.js
[add] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/xhr/open-url-redirected-sharedworker-origin.htm
[modify] https://crrev.com/739259e76c7efbacdd366642135e2c9c337dc470/third_party/WebKit/LayoutTests/external/wpt/xhr/open-url-redirected-worker-origin.htm

Comment 4 by hirosh...@chromium.org, Jul 10

Summary: Worker global scope's URL should be response URL (not request URL) (was: Worker's creation URL and base URL should be response URL (not request URL))
Created chromestatus entry: https://www.chromestatus.com/feature/4816360374796288
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 11 

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

commit b376ce6de5c36b8eafed8b6ad521e31effa2f63e
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed Jul 11 00:41:07 2018

Use non-null origin in threadable_loader_test

In order to assert |WorkerGlobalScope::url_| is non-empty
when referenced (which will be needed when Issue 861564 is
applied for module workers),
this CL removes cases where |url_| can be empty in unit tests.

Bug: 861564
Change-Id: I983f334ab866495a13a15e42805998359748b907
Reviewed-on: https://chromium-review.googlesource.com/1128518
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573997}
[modify] https://crrev.com/b376ce6de5c36b8eafed8b6ad521e31effa2f63e/third_party/blink/renderer/core/loader/threadable_loader_test.cc

Comment 6 by hirosh...@chromium.org, Jul 12

Blockedon: 863246

Comment 7 by hirosh...@chromium.org, Jul 23

Blockedon: 866666
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 26 

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

commit 026709ee4cedf9a30ae02cac76f54fd7ad2fb32f
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu Jul 26 18:20:52 2018

Reject cross-origin redirects for top-level classic worker scripts

This CL rejects cross-origin redirects in top-level scripts of
classic (dedicated | shared) workers in WorkerClassicScriptLoader,
to ensure that the response and request URLs of
a worker top-level script are same-origin in successful cases.

As most of cross-origin redirects should be already rejected
by loaders, this CL affects only cross-origin-redirected workers
created from an extension origin that has permissions and CSP
settings to access both the origins of the request and response URLs,
which I expect extremely rare.

This CL also adds browser_tests that cover such cases.

Bug: 861965, 861564
Change-Id: I80c27f9df550490384b5066b4b192d6415210b6d
Reviewed-on: https://chromium-review.googlesource.com/1135916
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578376}
[add] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/chrome/browser/extensions/worker_apitest.cc
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/chrome/test/BUILD.gn
[add] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/chrome/test/data/extensions/api_test/worker/background.js
[add] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/chrome/test/data/extensions/api_test/worker/manifest.json
[add] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/chrome/test/data/extensions/api_test/worker/shared_worker.js
[add] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/chrome/test/data/extensions/api_test/worker/worker.js
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/third_party/blink/renderer/core/workers/dedicated_worker.cc
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/third_party/blink/renderer/core/workers/worker_classic_script_loader.cc
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/third_party/blink/renderer/core/workers/worker_classic_script_loader.h
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/third_party/blink/renderer/modules/exported/web_embedded_worker_impl.cc
[modify] https://crrev.com/026709ee4cedf9a30ae02cac76f54fd7ad2fb32f/third_party/blink/renderer/modules/service_worker/web_embedded_worker_impl_test.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 26 

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

commit 316153bf20dfbe2dab7a8fad3fbc359584d68fe1
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu Jul 26 22:42:33 2018

Reject cross-origin redirects for top-level module worker scripts

This CL rejects cross-origin redirects in top-level scripts of
module dedicated workers in WorkerModuleScriptFetcher,
to ensure that the response and request URLs of
a worker top-level script are same-origin in successful cases.

Bug: 861965, 861564
Change-Id: I5969020480aeb0e1f108b0393199bb04b19419b6
Reviewed-on: https://chromium-review.googlesource.com/1147664
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578469}
[modify] https://crrev.com/316153bf20dfbe2dab7a8fad3fbc359584d68fe1/chrome/test/data/extensions/api_test/worker/background.js
[modify] https://crrev.com/316153bf20dfbe2dab7a8fad3fbc359584d68fe1/third_party/blink/renderer/core/loader/modulescript/worker_module_script_fetcher.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 27 

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

commit f2d4f9d463a01cdd194437a3f62f68cab9eadb7f
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jul 27 19:43:43 2018

Show origin for Paint/LayoutWorklet in DevTools context selector

Previously, an opaque origin is passed to
MainThreadDebugger::ContextCreated() for Paint/Layout worklets,
and caused the origins in the context selector shown as "IFrame".

This CL passes DocumentSecurityOrigin() instead, which is the
origin of the Document that creates the worklets, and thus
shows the correct origin.

Bug:  866666 , 861564
Change-Id: Ic038b3901ed0e301e4d2d9afdf632f7caf6add77
Reviewed-on: https://chromium-review.googlesource.com/1141290
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578754}
[modify] https://crrev.com/f2d4f9d463a01cdd194437a3f62f68cab9eadb7f/third_party/WebKit/LayoutTests/http/tests/devtools/console/paintworklet-console-selector-expected.txt
[modify] https://crrev.com/f2d4f9d463a01cdd194437a3f62f68cab9eadb7f/third_party/WebKit/LayoutTests/http/tests/devtools/console/paintworklet-console-selector.js
[modify] https://crrev.com/f2d4f9d463a01cdd194437a3f62f68cab9eadb7f/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc
[modify] https://crrev.com/f2d4f9d463a01cdd194437a3f62f68cab9eadb7f/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc

Comment 13 by falken@chromium.org, Jul 30 

These changes are great. Can we have a chromestatus entry for each separate web-exposed change, so we can track in which milestone changes were made. E.g,
- cross-origin redirects are no longer permitted for workers (created by extensions? module workers?)
- self.location is the response url after redirects (for classic dedicated and shared workers?)
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 1 

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

commit 50094223b687458f7ae9089bac76196d6ca296f3
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed Aug 01 04:44:27 2018

Remove dependency from WorkerThreadDebugger to global scope's Url()

Previously, WorkerThreadDebugger::ContextCreated() sets
V8ContextInfo using global scope's Url().
However, After [1] Url() can be null when the global scope is
created and thus ContextCreated() can't use Url().

This CL uses |GlobalScopeCreationParams::script_url| instead.
This CL shouldn't change the behavior, because currently
global scope's Url() is equal to
|GlobalScopeCreationParams::script_url|.

[1] https://chromium-review.googlesource.com/1139074

Bug:  866666 , 861564
Change-Id: Ic19a87d77c4b01d3f850e51ce734dae11e1b4e44
Reviewed-on: https://chromium-review.googlesource.com/1139052
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579702}
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/core/inspector/worker_thread_debugger.cc
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/core/inspector/worker_thread_debugger.h
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/core/workers/worker_thread.cc
[modify] https://crrev.com/50094223b687458f7ae9089bac76196d6ca296f3/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 2 

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

commit ed55940b959c7a156288114e7c8c3623cfcfb6fa
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu Aug 02 02:52:22 2018

Add tests for worker's URLs intercepted by ServiceWorker

This CL checks worker global scopes' URLs (self.location) in the
cases where worker top-level scripts are intercepted by
ServiceWorkers.

The test failure introduced in this CL will be fixed by
https://chromium-review.googlesource.com/1153598.

Bug: 861564
Change-Id: Ia92e1de697b5b9c6a61ab8e5c5abcaaf6dcee777
Reviewed-on: https://chromium-review.googlesource.com/1157220
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580041}
[modify] https://crrev.com/ed55940b959c7a156288114e7c8c3623cfcfb6fa/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/worker-interception-redirect-webworker.js
[modify] https://crrev.com/ed55940b959c7a156288114e7c8c3623cfcfb6fa/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/worker-interception-redirect.https-expected.txt
[modify] https://crrev.com/ed55940b959c7a156288114e7c8c3623cfcfb6fa/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/worker-interception-redirect.https.html
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 17 

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

commit 045b8f9de7dd287a15613af65eb45b93b530adfb
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Aug 17 09:49:35 2018

Use OriginalURLViaServiceWorker() for classic worker scripts

This CL changes worker global scope's URL if the request of
the worker top-level script is intercepted by ServiceWorker and
Response with a different URL is supplied to respondWith().

Test: external/wpt/service-workers/service-worker/worker-interception-redirect.https.html
Bug: 861564
Change-Id: I4b91777abd6fa42583ae853330ee2c7bbccf61cb
Reviewed-on: https://chromium-review.googlesource.com/1153598
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584003}
[modify] https://crrev.com/045b8f9de7dd287a15613af65eb45b93b530adfb/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/worker-interception-redirect.https-expected.txt
[modify] https://crrev.com/045b8f9de7dd287a15613af65eb45b93b530adfb/third_party/blink/renderer/core/workers/worker_classic_script_loader.cc

Comment 17 by hirosh...@chromium.org, Aug 28

Blockedon: 878274

Comment 18 by hirosh...@chromium.org, Sep 3

Blocking: 680046

Comment 19 by hirosh...@chromium.org, Nov 14

Blockedon: -863246
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 12 

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

commit 6742d7a6afddc623f0e9f4bede5a8d35b83dc802
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed Dec 12 02:29:43 2018

Fix cross-origin redirect check in WorkerClassicScriptLoader

https://chromium-review.googlesource.com/1153598 switched
WorkerClassicScriptLoader::ResponseURL() (which is used as
WorkerGlobalScope's URL later)
from ResourceResponse::CurrentRequestUrl()
to ResourceResponse::ResponseUrl() (in terms of the current ToT),
but the same-origin check for Issue 861965 remained
unchanged, i.e. checking ResourceResponse::CurrentRequestUrl(),
which doesn't reflect ServiceWorker interception.

This CL updates the same-origin check to also check
ResourceResponse::ResponseUrl(), to ensure
WorkerGlobalScope's URL and SecurityOrigin always match
even if a ServiceWorker returns a response with a cross-origin URL.

Bug: 861965, 861564
Change-Id: I9915599dc1ee16dbd4879525db90c4cb38da4eb3
Reviewed-on: https://chromium-review.googlesource.com/c/1368460
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615787}
[modify] https://crrev.com/6742d7a6afddc623f0e9f4bede5a8d35b83dc802/third_party/blink/renderer/core/workers/worker_classic_script_loader.cc
Project Member

Comment 21 by bugdroid1@chromium.org, Jan 11 

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

commit c3408668fb2bb8bce5bb6b0dc2a00ec09c1e6419
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jan 11 02:01:51 2019

Do not call EnsureFetcher() in ThreadableLoader for outsideSettings fetch

Currently, ThreadableLoader always calls EnsureFetcher(),
which is not necessary and
would cause assertion failures when switching the worker's URL
because this creates worker's insideSettings ResourceFetcher
(which depends on ExecutionContext and its URL)
at the start of outsideSettings top-level script fetch.

This CL skips such unnecessary EnsureFetcher() calls and thus
avoids the assertion failures.

Bug: 880027, 835717, 861564
Change-Id: I7768b0274bdc47e856df2a4a529d4b223d978c80
Reviewed-on: https://chromium-review.googlesource.com/c/1394643
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621863}
[modify] https://crrev.com/c3408668fb2bb8bce5bb6b0dc2a00ec09c1e6419/third_party/blink/renderer/core/loader/threadable_loader.cc
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 15 

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

commit 64c8be686b405ef50b2e6a8ac9af583465d3a2eb
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue Jan 15 02:59:12 2019

Introduce OffMainThreadWorkerScriptFetchOption

Instead of WorkerGlobalScope::IsScriptFetchedOnMainThread()
that doesn't work in the constructors, this CL introduces
GlobalScopeCreationParams::off_main_thread_fetch_option.

Bug:  917932 , 861564
Change-Id: I325b7bbe8400019ef182e95035a5eb1283bb23d4
Reviewed-on: https://chromium-review.googlesource.com/c/1394192
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622698}
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope_proxy.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/dedicated_worker.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/dedicated_worker.h
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/dedicated_worker_messaging_proxy.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/dedicated_worker_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/experimental/thread_pool.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/global_scope_creation_params.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/global_scope_creation_params.h
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/main_thread_worklet_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/worker_global_scope.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/worker_global_scope.h
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/worker_thread_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_proxy.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/exported/web_embedded_worker_impl.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/webaudio/audio_worklet_thread_test.cc
[modify] https://crrev.com/64c8be686b405ef50b2e6a8ac9af583465d3a2eb/third_party/blink/renderer/modules/worklet/animation_and_paint_worklet_thread_test.cc
Project Member

Comment 23 by bugdroid1@chromium.org, Jan 15 

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

commit 386065dc11ff8a2184fc5a1d772e342fd312736e
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Tue Jan 15 05:32:03 2019

Use HttpsState in MixedContentChecker::ShouldAutoupgrade()

This CL replaces a call to ExecutionContext::Url() with
GetHttpsState(), in order to

1. Remove Url() calls during fetch for off-the-main-thread
   worker top-level script fetch (crbug.com/861564), and
2. Make auto-upgrade consistent with other parts of
   MixedContentChecker.
   After this CL, both mixed content check and auto upgrade
   are based on SecurityOrigin on Documents
   (Frame's SecurityContext's SecurityOrigin or
   Document's SecurityOrigin via Document::GetHttpsState()),
   and based on HttpsState on workers/worklets.

This changes the behavior of auto upgrade for requests from
blank <iframe>s and sandboxed <iframe>s, where
Document::Url()'s origin and Document::GetSecurityOrigin()
are different.

Bug: 861564, 893647
Change-Id: Ie793d8fb0f848d84ff9468663639ab480027d7b4
Reviewed-on: https://chromium-review.googlesource.com/c/1394371
Reviewed-by: Adam Rice <ricea@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622743}
[modify] https://crrev.com/386065dc11ff8a2184fc5a1d772e342fd312736e/third_party/blink/renderer/core/loader/frame_fetch_context_test.cc
[modify] https://crrev.com/386065dc11ff8a2184fc5a1d772e342fd312736e/third_party/blink/renderer/core/loader/frame_loader.cc
[modify] https://crrev.com/386065dc11ff8a2184fc5a1d772e342fd312736e/third_party/blink/renderer/core/loader/mixed_content_checker.cc
[modify] https://crrev.com/386065dc11ff8a2184fc5a1d772e342fd312736e/third_party/blink/renderer/core/loader/mixed_content_checker.h
[modify] https://crrev.com/386065dc11ff8a2184fc5a1d772e342fd312736e/third_party/blink/renderer/modules/websockets/dom_websocket.cc
[modify] https://crrev.com/386065dc11ff8a2184fc5a1d772e342fd312736e/third_party/blink/renderer/modules/websockets/dom_websocket_test.cc
Project Member

Comment 24 by bugdroid1@chromium.org, Jan 15 

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

commit 4561166b9e8eb9e1f704b51a8961e4d31fc85bf6
Author: Dominic Battré <battre@chromium.org>
Date: Tue Jan 15 08:31:45 2019

Revert "Introduce OffMainThreadWorkerScriptFetchOption"

This reverts commit 64c8be686b405ef50b2e6a8ac9af583465d3a2eb.

Reason for revert: Breaks linux-jumbo-rel see  crbug.com/917932 

Original change's description:
> Introduce OffMainThreadWorkerScriptFetchOption
> 
> Instead of WorkerGlobalScope::IsScriptFetchedOnMainThread()
> that doesn't work in the constructors, this CL introduces
> GlobalScopeCreationParams::off_main_thread_fetch_option.
> 
> Bug:  917932 , 861564
> Change-Id: I325b7bbe8400019ef182e95035a5eb1283bb23d4
> Reviewed-on: https://chromium-review.googlesource.com/c/1394192
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#622698}

TBR=haraken@chromium.org,hiroshige@chromium.org,nhiroki@chromium.org

Change-Id: I8692c0ef29d997d6f4dff658ffd7160a7ab35101
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  917932 , 861564
Reviewed-on: https://chromium-review.googlesource.com/c/1411592
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622780}
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope_proxy.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/dedicated_worker.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/dedicated_worker.h
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/dedicated_worker_messaging_proxy.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/dedicated_worker_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/experimental/thread_pool.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/global_scope_creation_params.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/global_scope_creation_params.h
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/main_thread_worklet_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/worker_global_scope.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/worker_global_scope.h
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/worker_thread_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_proxy.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/exported/web_embedded_worker_impl.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/webaudio/audio_worklet_thread_test.cc
[modify] https://crrev.com/4561166b9e8eb9e1f704b51a8961e4d31fc85bf6/third_party/blink/renderer/modules/worklet/animation_and_paint_worklet_thread_test.cc

Comment 25 by falken@chromium.org, Jan 17 (6 days ago)

Blocking: 553535
Project Member

Comment 26 by bugdroid1@chromium.org, Jan 18 (5 days ago) 

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

commit 28103b0a84ba67d3a4a289fdfee475d3aa85dd27
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Fri Jan 18 02:57:25 2019

Reland "Introduce OffMainThreadWorkerScriptFetchOption"

This reverts commit 4561166b9e8eb9e1f704b51a8961e4d31fc85bf6.

Reason for revert: The failure was probably due to
 crbug.com/921967  and the issue was fixed.

Original change's description:
> Revert "Introduce OffMainThreadWorkerScriptFetchOption"
> 
> This reverts commit 64c8be686b405ef50b2e6a8ac9af583465d3a2eb.
> 
> Reason for revert: Breaks linux-jumbo-rel see  crbug.com/917932 
> 
> Original change's description:
> > Introduce OffMainThreadWorkerScriptFetchOption
> > 
> > Instead of WorkerGlobalScope::IsScriptFetchedOnMainThread()
> > that doesn't work in the constructors, this CL introduces
> > GlobalScopeCreationParams::off_main_thread_fetch_option.
> > 
> > Bug:  917932 , 861564
> > Change-Id: I325b7bbe8400019ef182e95035a5eb1283bb23d4
> > Reviewed-on: https://chromium-review.googlesource.com/c/1394192
> > Reviewed-by: Kentaro Hara <haraken@chromium.org>
> > Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
> > Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#622698}
> 
> TBR=haraken@chromium.org,hiroshige@chromium.org,nhiroki@chromium.org
> 
> Change-Id: I8692c0ef29d997d6f4dff658ffd7160a7ab35101
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  917932 , 861564
> Reviewed-on: https://chromium-review.googlesource.com/c/1411592
> Reviewed-by: Dominic Battré <battre@chromium.org>
> Commit-Queue: Dominic Battré <battre@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#622780}

TBR=battre@chromium.org,haraken@chromium.org,hiroshige@chromium.org,nhiroki@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  917932 , 861564
Change-Id: I77ef643ca28c7a0fb69c88e76238652e75f8d226
Reviewed-on: https://chromium-review.googlesource.com/c/1419371
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623975}
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope_proxy.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/dedicated_worker.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/dedicated_worker.h
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/dedicated_worker_messaging_proxy.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/dedicated_worker_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/experimental/thread_pool.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/global_scope_creation_params.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/global_scope_creation_params.h
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/main_thread_worklet_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/threaded_worklet_messaging_proxy.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/worker_global_scope.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/worker_global_scope.h
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/worker_thread_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_proxy.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/exported/web_embedded_worker_impl.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/webaudio/audio_worklet_thread_test.cc
[modify] https://crrev.com/28103b0a84ba67d3a4a289fdfee475d3aa85dd27/third_party/blink/renderer/modules/worklet/animation_and_paint_worklet_thread_test.cc
►

Sign in to add a comment