1793995 - Factor parts of WebExtensionPolicy into a threadsafe core type

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

A way to reference a WebExtensionPolicy from any thread will be necessary to make nsIPrincipal threadsafe (bug 1443925). It will both be needed as part of implementing ContentPrincipal::AddonPolicy() as well as part of supporting NS_URIChainHasFlags on any thread (bug 1793463).

The particular fields which we want to be make threadsafe includes WebAccessibleResource in order to implement NS_URIChainHasFlags. This will require replacing the regex backend used by MatchGlob from using the JS engine, so that it can be used on any thread. This will require us to switch to the rust regex library (bug 1693271), as well as split out the WebIDL wrappers for the various types (MatchGlob, MatchPattern, MatchPatternSet), so that the core types can be made threadsafe.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 1: Move immutable parts of WebExtensionPolicy to a threadsafe core type, r=kmag!

This threadsafe core type also acts as a weak reference to the main-thread
WebExtensionPolicy when needed. This will be used when information about a
WebExtension is needed to be accessible off-main-thread in the future.

Depends on D158878

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 2: Make AtomSet immutable and threadsafe, r=kmag!

The mutator methods were never used, so were easy to remove, allowing the type
to be made threadsafe. The main potential for performance regression is that
the string based Contains method now uses NS_Atomize instead of
NS_AtomizeMainThread, however this is only called in once place
(WebExtensionPolicy::HasPermission). If it turns out to be an issue, we can
move the atomization into the caller to keep it using NS_AtomizeMainThread.

Depends on D158879

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 3: Remove the unused includeGlobs and excludeGlobs getters, r=kmag!

These getters are never called and will make making the core of MatchGlob
threadsafe more annoying.

Depends on D158880

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 4: Split out the threadsafe core from MatchGlob, r=kmag!

The outer cycle-collected wrapper type is unfortunately still required by
WebIDL in order to keep the JS API working.

Depends on D158881

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 5: Factor out the threadsafe core of MatchPattern and MatchPatternSet, r=kmag!

Similar to MatchGlob, this is fairly straightforward, but is complicated
slightly by MatchPatternSet, which allows being destructured into the
contained MatchPattern instances in a [Constant] method. To handle this a
cache is added for the wrappers in the cycle-collected MatchPatternSet.

Depends on D158882

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 6: Allow looking up a WebExtensionPolicyCore from any thread, r=kmag!

In order to make WebAccessibleResource threadsafe, as well as other places, it
needs to be possible to look up a WebExtensionPolicyCore from any thread.
This is handled by using a static method on the ExtensionPolicyService for this
task, and keeping a seperate mutex-guarded static table under the hood.

Theoretically the table within ExtensionPolicyService() could also be
removed, however I held off on doing that in case it would have a negative
performance impact to take extra locks and follow extra pointers.

Depends on D158883

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 7: Use threadsafe refcounting for WebAccessibleResource, r=kmag!

Now that all fields and methods in WebAccessibleResource have been made
threadsafe, we can make the type itself be threadsafe.

Depends on D158884

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1793995 - Part 8: Move WebAccessibleResources into the threadsafe core, r=kmag!

This will be required in the future to make getting protocol flags for
moz-extension:// URIs threadsafe.

Depends on D158885

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cb4daddadb9
Part 1: Move immutable parts of WebExtensionPolicy to a threadsafe core type, r=kmag
https://hg.mozilla.org/integration/autoland/rev/06d7d4823419
Part 2: Make AtomSet immutable and threadsafe, r=kmag
https://hg.mozilla.org/integration/autoland/rev/37ccdfa60f9a
Part 3: Remove the unused includeGlobs and excludeGlobs getters, r=kmag
https://hg.mozilla.org/integration/autoland/rev/f7d78fffc836
Part 4: Split out the threadsafe core from MatchGlob, r=kmag
https://hg.mozilla.org/integration/autoland/rev/8484584cc787
Part 5: Factor out the threadsafe core of MatchPattern and MatchPatternSet, r=kmag
https://hg.mozilla.org/integration/autoland/rev/61d74f2cf5cf
Part 6: Allow looking up a WebExtensionPolicyCore from any thread, r=kmag
https://hg.mozilla.org/integration/autoland/rev/3deec78af656
Part 7: Use threadsafe refcounting for WebAccessibleResource, r=kmag
https://hg.mozilla.org/integration/autoland/rev/93fa076646e1
Part 8: Move WebAccessibleResources into the threadsafe core, r=kmag

Comment 10

[Cristian Tuns]

Backed out for causing build bustages on RustRegex.h

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/workspace/obj-build/dist/include/mozilla/RustRegex.h:630:24: error: unused variable 'view' [-Werror,-Wunused-variable]

Log tier 2: https://treeherder.mozilla.org/logviewer?job_id=392996025&repo=autoland

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16299839e25e
Part 1: Move immutable parts of WebExtensionPolicy to a threadsafe core type, r=kmag
https://hg.mozilla.org/integration/autoland/rev/d55a78f3627a
Part 2: Make AtomSet immutable and threadsafe, r=kmag
https://hg.mozilla.org/integration/autoland/rev/6773cada61a8
Part 3: Remove the unused includeGlobs and excludeGlobs getters, r=kmag
https://hg.mozilla.org/integration/autoland/rev/264a0154514e
Part 4: Split out the threadsafe core from MatchGlob, r=kmag
https://hg.mozilla.org/integration/autoland/rev/a653699a5cef
Part 5: Factor out the threadsafe core of MatchPattern and MatchPatternSet, r=kmag
https://hg.mozilla.org/integration/autoland/rev/4330821df43f
Part 6: Allow looking up a WebExtensionPolicyCore from any thread, r=kmag
https://hg.mozilla.org/integration/autoland/rev/79a5023e7822
Part 7: Use threadsafe refcounting for WebAccessibleResource, r=kmag
https://hg.mozilla.org/integration/autoland/rev/5d4b0c23342f
Part 8: Move WebAccessibleResources into the threadsafe core, r=kmag

Comment 12

[Cristian Tuns]

Backed out for causing build bustages

• Backout link
• Push with failures
• Failure Log
• Failure line: clang-14: error: linker command failed with exit code 1 (use -v to see invocation)
  gmake[4]: *** [/builds/worker/checkouts/gecko/config/makefiles/rust.mk:433: force-cargo-library-build] Error 101

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/61417ba55b4c
Part 1: Move immutable parts of WebExtensionPolicy to a threadsafe core type, r=kmag
https://hg.mozilla.org/integration/autoland/rev/ef9dcb4d17d2
Part 2: Make AtomSet immutable and threadsafe, r=kmag
https://hg.mozilla.org/integration/autoland/rev/f1d329c29295
Part 3: Remove the unused includeGlobs and excludeGlobs getters, r=kmag
https://hg.mozilla.org/integration/autoland/rev/70fb967a388d
Part 4: Split out the threadsafe core from MatchGlob, r=kmag
https://hg.mozilla.org/integration/autoland/rev/9e563403fa06
Part 5: Factor out the threadsafe core of MatchPattern and MatchPatternSet, r=kmag
https://hg.mozilla.org/integration/autoland/rev/6fb4337b5bc0
Part 6: Allow looking up a WebExtensionPolicyCore from any thread, r=kmag
https://hg.mozilla.org/integration/autoland/rev/a891737169cc
Part 7: Use threadsafe refcounting for WebAccessibleResource, r=kmag
https://hg.mozilla.org/integration/autoland/rev/00ee36d3c47b
Part 8: Move WebAccessibleResources into the threadsafe core, r=kmag

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/61417ba55b4c
https://hg.mozilla.org/mozilla-central/rev/ef9dcb4d17d2
https://hg.mozilla.org/mozilla-central/rev/f1d329c29295
https://hg.mozilla.org/mozilla-central/rev/70fb967a388d
https://hg.mozilla.org/mozilla-central/rev/9e563403fa06
https://hg.mozilla.org/mozilla-central/rev/6fb4337b5bc0
https://hg.mozilla.org/mozilla-central/rev/a891737169cc
https://hg.mozilla.org/mozilla-central/rev/00ee36d3c47b