1711168 - support extensions in web_accessible_resources

Status: RESOLVED FIXED

(WebExtensions :: General, enhancement, P2)

Description

[Shane Caraveo (:mixedpuppy)]

Add support for extension id matching in WAC, and require either matches or extensions, not both.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1711168 support extension matching in webAccessibleResources

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19de2822bc0c
support extension matching in webAccessibleResources r=zombie,smaug

Comment 3

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/19de2822bc0c

Comment 4

[Alexandru Michis [:malexandru]]

Backed out changeset 19de2822bc0c (bug 1711168) for causing Bug 1719063.
https://hg.mozilla.org/integration/autoland/rev/8fc06b7667d8102c267b236488da6bbc39ef962e

Comment 5

[Shane Caraveo (:mixedpuppy)]

enforcing a restriction on extensions opening other extension resources breaks newtab, homepage, probably protocol handlers. In order to enforce such a restriction, we need to flag these "ui resources" somehow, perhaps like we do for some about pages using nsIAboutModule::MAKE_LINKABLE or nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT. Another option would be to set nsIProtocolHandler::URI_LOADABLE_BY_ANYONE in the extension protocol handler.

Comment 6

[Alexandru Michis [:malexandru]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/c2757388a512ccf02cf172dffb9da01203ce3099

Comment 7

[Shane Caraveo (:mixedpuppy)]

Another thought is to bypass the extension match restriction if BrowsingContext->isTop. That (I think) should allow something like a tab manager to open tabs to other extensions, but require matching for embedding iframes/etc.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:mixedpuppy, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 9

[Tomislav Jovanovic :zombie]

Found issues with the patch on try, it's being worked on.

Comment 10

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1711168 allow any extension to access other extension ui resources like homepage/newtab

A special exception is made to allow certain resource to be accessed by any extenison.
This allows e.g. a tab manager to open a newtab that is provided by a different extension.

Comment 11

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1711168 ensure extension provided newtab and homepage works with webAccessibleResource restrictions

Comment 12

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1711168 allow extension pages to be loaded as top level tabs by other extensions

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d061ce03de7
support extension matching in webAccessibleResources r=zombie,smaug,rpl
https://hg.mozilla.org/integration/autoland/rev/902385a7ce60
allow extension pages to be loaded as top level tabs by other extensions r=rpl

Comment 14

[Atila Butkovits]

Backed out 3 for causing failures at test_ext_web_accessible_resources.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/afb50a831d7ac9e372bc71f0aeb796e479151366

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=R-B-cHihRMO2Z-90D9k-dQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=703c909eb009db8f549f68fc0b6a5c78ef6493f3

Failure log:

• https://treeherder.mozilla.org/logviewer?job_id=385487852&repo=autoland&lineNumber=6171
• https://treeherder.mozilla.org/logviewer?job_id=385487172&repo=autoland&lineNumber=7509

Comment 15

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Atila Butkovits from comment #14)

Backed out 3 for causing failures at test_ext_web_accessible_resources.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/afb50a831d7ac9e372bc71f0aeb796e479151366

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=R-B-cHihRMO2Z-90D9k-dQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=703c909eb009db8f549f68fc0b6a5c78ef6493f3

Failure log:

• https://treeherder.mozilla.org/logviewer?job_id=385487852&repo=autoland&lineNumber=6171
• https://treeherder.mozilla.org/logviewer?job_id=385487172&repo=autoland&lineNumber=7509

== Change summary for alert #34946 (as of Thu, 28 Jul 2022 08:05:53 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
14%	google-mail loadtime	macosx1015-64-shippable-qr	fission warm webrender	618.98 -> 533.04
10%	cnn fcp	linux1804-64-shippable-qr	bytecode-cached cold fission webrender	869.31 -> 778.88
9%	google-mail loadtime	macosx1015-64-shippable-qr	cold fission webrender	1,392.60 -> 1,266.00
9%	google-docs FirstVisualChange	macosx1015-64-shippable-qr	cold fission webrender	758.33 -> 690.00
6%	cnn SpeedIndex	linux1804-64-shippable-qr	bytecode-cached cold fission webrender	1,062.00 -> 994.67
...	...	...	...	...
3%	buzzfeed PerceptualSpeedIndex	macosx1015-64-shippable-qr	cold fission webrender	696.12 -> 674.83

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=34946

Comment 16

[Shane Caraveo (:mixedpuppy)]

Alexandru,
I'm trying to understand what the perf improvements are. From what I can see, the values for the commit in comment 13 are all in line with previous values. The backout seems to be improving overal perf, but that doesn't make sense. The google-docs value is an clear example. Backout and report in comment 15 says a 9% improvement. But the prior entry is my patch, which is 760ms, which is what almost all prior tests have.

I'm not sure how I'm supposed to think about this.

Comment 17

[Shane Caraveo (:mixedpuppy)]

I've looked over the perfherder graphs and our patches with another team member. We don't see how the changes have any relation or affect and given the patch that landed had the same timing as prior patches, we don't think the "improvement" is related to the patch and is otherwise a coincidence. I'm going to reland and see how this looks after.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4cfd51c07734
support extension matching in webAccessibleResources r=zombie,smaug,rpl
https://hg.mozilla.org/integration/autoland/rev/d0da1be14163
allow extension pages to be loaded as top level tabs by other extensions r=rpl,geckoview-reviewers,calu

Comment 19

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/4cfd51c07734
https://hg.mozilla.org/mozilla-central/rev/d0da1be14163

Comment 20

[Rob Wu [:robwu]]

Considering the backwards-breaking change in this bug, we should add a note to the changelog at https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/105:

Extensions resources can no longer be loaded by other extensions by default, unless the extension resource is listed in web_accessible_resources.

Comment 21

[rbloor]

Documentation updates completed in PR #21194 and browser compatibility data changes in PR #17914