New issue
Advanced search Search tips

Issue 882467 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Task

Blocked on:
issue 835717

Blocking:
issue 820329


Participants' hotlists:
Worker-OffTheMainThread


Sign in to add a comment

Remove dependencies on WorkerShadowPage for WorkerInspectorProxy

Project Member Reported by nhiroki@chromium.org, Sep 10

Issue description

In the current implementation, out-of-process workers (i.e., shared workers and service workers) require WorkerShadowPage on the main thread in order to provide a dummy Document to WorkerInspectorProxy (see WebEmbeddedWorkerImpl::StartWorkerThread() for example). This dependency should be removed for enabling the workers to start off the main thread (issue 820329)
 
Owner: dgozman@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 31

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

commit 1382cbcc0e916f8449d471dd54ca62aeb14db252
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Wed Oct 31 23:42:42 2018

[DevTools] (Re-)introduce SDK.Target.Type

We failed to abstract away the types with capabilities, so
I am bringing the type back in preparation of removing
service/shadow workers shadow page target.

Bug: 882467
Change-Id: I480bb18b1cfa9cf79e83435d735d102711991562
Reviewed-on: https://chromium-review.googlesource.com/c/1310759
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604446}
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger-ui/last-execution-context.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/WebKit/LayoutTests/http/tests/devtools/unit/node-url.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/console/ConsoleContextSelector.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/inspector_main/InspectorMain.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/inspector_main/RequestAppBannerActionDelegate.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/js_main/JsMain.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/main/Main.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/node_main/NodeMain.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/persistence/Automapping.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/persistence/Persistence.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/protocol/InspectorBackend.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/protocol/NodeURL.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/resources/AppManifestView.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk/ChildTargetManager.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk/DebuggerModel.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk/RuntimeModel.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk/ServiceWorkerManager.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk/Target.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk/TargetManager.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sdk_test_runner/PageMockTestRunner.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/sources/NavigatorView.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/test_runner/TestRunner.js
[modify] https://crrev.com/1382cbcc0e916f8449d471dd54ca62aeb14db252/third_party/blink/renderer/devtools/front_end/worker_main/WorkerMain.js

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 1

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

commit 104631103c9be092fdae2d299c9a24b85e39f3d5
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Thu Nov 01 02:45:03 2018

[DevTools] Put worker initalization params to WorkerDevToolsParams

This simple struct is being used to initialize DevTools in workers.

Bug: 882467
Change-Id: I4d7fa582913b3be9a8466de24d727289a34f6ce0
Reviewed-on: https://chromium-review.googlesource.com/c/1308073
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604495}
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/exported/web_shared_worker_impl.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/inspector/BUILD.gn
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/inspector/devtools_agent.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/inspector/devtools_agent.h
[add] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/inspector/worker_devtools_params.h
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/inspector/worker_inspector_controller.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/inspector/worker_inspector_controller.h
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/workers/threaded_messaging_proxy_base.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/workers/worker_thread.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/workers/worker_thread.h
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/workers/worker_thread_test.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/core/workers/worker_thread_test_helper.h
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope_test.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/modules/exported/web_embedded_worker_impl.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope_test.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/modules/webaudio/audio_worklet_thread_test.cc
[modify] https://crrev.com/104631103c9be092fdae2d299c9a24b85e39f3d5/third_party/blink/renderer/modules/worklet/animation_and_paint_worklet_thread_test.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 1

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

commit 566b6b5d47bc4fb26fb026e66ef2c62b759562ca
Author: Dmitry Gozman <dgozman@chromium.org>
Date: Thu Nov 01 15:58:15 2018

[DevTools] Cleanup after introducing SDK.Target.Type

We don't need some test runner methods anymore.

We can also determine capabilities by type instead of passing
them separately.

Bug: 882467
Change-Id: I0dc07f4379141905d41968f969650f3d794fdee2
Reviewed-on: https://chromium-review.googlesource.com/c/1311839
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604598}
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/elements/event-listeners-framework-with-service-worker.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-manager-expected.txt
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-manager.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/user-agent-override.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger-ui/last-execution-context.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger-ui/scripts-sorting.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/sources/debugger/navigator-view.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/startup/dedicated-workers-list.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/WebKit/LayoutTests/http/tests/devtools/workers-on-navigation.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/application_test_runner/ServiceWorkersTestRunner.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/inspector_main/InspectorMain.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/js_main/JsMain.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/node_main/NodeMain.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/sdk/ChildTargetManager.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/sdk/Target.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/sdk/TargetManager.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/sdk_test_runner/PageMockTestRunner.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/test_runner/TestRunner.js
[modify] https://crrev.com/566b6b5d47bc4fb26fb026e66ef2c62b759562ca/third_party/blink/renderer/devtools/front_end/worker_main/WorkerMain.js

Cc: nhiroki@chromium.org
@nhiroki: I have a patch mostly working for this. However, it seems like we are missing the main script, since it's issued on the main thread, not on the worker thread. This blocks me from landing the patch. Is there ongoing work to move request for the worker script to the worker thread itself?
Cc: hirosh...@chromium.org
Status: Started (was: Assigned)
dgozman@: Thank you for working on this!

Yes. hiroshige@ and I are now working on off-the-main-thread worker script fetch (issue 835717). Our first target is dedicated workers that don't use worker shadow page. Once it's complete (hopefully in Q4), we'll implement it for shared workers and service workers, too.
Blockedon: 835717
The migration patch is here: https://chromium-review.googlesource.com/c/chromium/src/+/1308981.

Sign in to add a comment