Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSE-in-Workers: Cross-thread registry with stubbed attachment #25609

Merged
merged 1 commit into from Sep 24, 2020

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Sep 18, 2020

Introduces a concrete CrossThreadMediaSourceAttachment with minimum
implementation necessary to enable worker thread MediaSource object URL
creation. To enable registration of worker-thread MediaSource object
URLs, which is inherent in the createObjectURL implementation, updates
MediaSourceRegistryImpl to perform registration and unregistration while
holding a mutex in the singleton, main-thread-owned, registry instance.
Mutexes previously were thought unusable for this, but that was due to
the registry previously using an Oilpan HeapHashMap bound to the main
thread's Oilpan heap. The current registry now uses a non-Oilpan
HashMap, so any thread can update the singleton so long as such
accesses or updates are mutex-protected. Also, using cross-thread task
posting to perform these tasks led to complex races whose solution is
much simpler using the mutex approach herein. See
CrossThreadMediaSourceAttachment::Unregister() for more detail.

Includes a necessary update which makes both types of attachments manage
the registered media source in appropriate Oilpan type: the cross thread
attachment must hold that reference as CrossThreadPersistent. The URL
registry implementation already unregisters all entries created on an
execution context if that context is destroyed, so this
CrossThreadPersistent registry entry will not outlive the worker
thread's context. See PublicURLManager::ContextDestroyed() for where
that logic exists already. Rationale for not also making the
SameThreadMediaSourceAttachment use a CrossThreadPersistent type is that
such type introduces a new root in all Oilpan heaps, and resulting
performance hit can be avoided by just using regular Persistents for
same thread attachments' |registered_media_source_|.

Includes new web_tests that exercise basic worker context MediaSource
construction, object URL creation and revocation (with revocation of
worker MediaSource object URL also tested on main thread). Starting an
attachment to a worker MediaSource is also tested, but is currently
expected by the test to fail until upcoming
CrossThreadMediaSourceAttachment changes land.

BUG=878133

Change-Id: I367b6610da9aca3aca7c78f4a11f571e48afc6c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407075
Reviewed-by: Will Cassella <cassew@google.com>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810002}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-2407075 branch 2 times, most recently from 1af420e to 6330599 Compare September 23, 2020 20:12
Introduces a concrete CrossThreadMediaSourceAttachment with minimum
implementation necessary to enable worker thread MediaSource object URL
creation. To enable registration of worker-thread MediaSource object
URLs, which is inherent in the createObjectURL implementation, updates
MediaSourceRegistryImpl to perform registration and unregistration while
holding a mutex in the singleton, main-thread-owned, registry instance.
Mutexes previously were thought unusable for this, but that was due to
the registry previously using an Oilpan HeapHashMap bound to the main
thread's Oilpan heap. The current registry now uses a non-Oilpan
HashMap, so any thread can update the singleton so long as such
accesses or updates are mutex-protected. Also, using cross-thread task
posting to perform these tasks led to complex races whose solution is
much simpler using the mutex approach herein. See
CrossThreadMediaSourceAttachment::Unregister() for more detail.

Includes a necessary update which makes both types of attachments manage
the registered media source in appropriate Oilpan type: the cross thread
attachment must hold that reference as CrossThreadPersistent. The URL
registry implementation already unregisters all entries created on an
execution context if that context is destroyed, so this
CrossThreadPersistent registry entry will not outlive the worker
thread's context. See PublicURLManager::ContextDestroyed() for where
that logic exists already. Rationale for not also making the
SameThreadMediaSourceAttachment use a CrossThreadPersistent type is that
such type introduces a new root in all Oilpan heaps, and resulting
performance hit can be avoided by just using regular Persistents for
same thread attachments' |registered_media_source_|.

Includes new web_tests that exercise basic worker context MediaSource
construction, object URL creation and revocation (with revocation of
worker MediaSource object URL also tested on main thread). Starting an
attachment to a worker MediaSource is also tested, but is currently
expected by the test to fail until upcoming
CrossThreadMediaSourceAttachment changes land.

BUG=878133

Change-Id: I367b6610da9aca3aca7c78f4a11f571e48afc6c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407075
Reviewed-by: Will Cassella <cassew@google.com>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810002}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants