1416343 - Mark nsIURI and interfaces extending it as [builtinclass]

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P3)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd2603f02f956923c2e3c0aee49e49e0d5c9918a

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1416343 - Mark URI interfaces as [builtinclass]

Review commit: https://reviewboard.mozilla.org/r/199174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/199174/

Comment 3

[Daniel Stenberg [:bagder]]

Comment on attachment 8927933 [details]
Bug 1416343 - Mark URI interfaces as [builtinclass]

https://reviewboard.mozilla.org/r/199174/#review204618

LGTM, just one little minor things...

::: browser/modules/test/browser/browser_SitePermissions_tab_urls.js:10
(Diff revision 1)
>  "use strict";
>  
>  Cu.import("resource:///modules/SitePermissions.jsm", this);
>  
> +function uri(url) {
> +  return Services.io.newURI(url);

nit: this seems to use a smaller indent than other parts of this file?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8927933 [details]
Bug 1416343 - Mark URI interfaces as [builtinclass]

https://reviewboard.mozilla.org/r/199174/#review204618

> nit: this seems to use a smaller indent than other parts of this file?

This file has several indentation types. I'm going to stick with 2 spaces for now.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cfe4b1e6be9b
Mark URI interfaces as [builtinclass] r=bagder

Comment 6

[Eliza Balazs [:ebalazs_]]

Backed out changeset cfe4b1e6be9b (bug 1416343) for ESlint failing in modules/test/browser/browser_SitePermissions_tab_urls.js:38:14 r=backout on a CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=cfe4b1e6be9bb8fe780209776f03fb0c834bafa0&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable&selectedJob=144907308

https://hg.mozilla.org/integration/autoland/rev/561d658b088d1ea00d226891603b91acadee4d91

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8927933 [details]
Bug 1416343 - Mark URI interfaces as [builtinclass]

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/199174/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fcc3fb66b12
Mark URI interfaces as [builtinclass] r=bagder

Comment 9

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/5fcc3fb66b12

Comment 10

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f72846b0b33a8219d79b22d3be73916819661d8f

Comment 11

[Jorg K (CEST = GMT+2)]

What was the motivation for this change? It's breaking Thunderbird quite badly since we rely on being able to define a URL class with is derived from nsIURI in JS.

In fact, Thunderbird offers a facility to define mail account types in JS (called JS Account). Since TB relies on URLs to address almost everything, mail messages, servers, etc. using various nsIURI-derived classes, this JS Account module is now broken.

Comment 12

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jorg K [Almost not working on Thunderbird (some bustage-fix only) due to non-renewal of contract] from comment #11)
> What was the motivation for this change? It's breaking Thunderbird quite
> badly since we rely on being able to define a URL class with is derived from
> nsIURI in JS.

This is a necessary step in our plan to make nsIURI thread safe (bug 922464). Being implemented in JS makes it impossible to be used OMT.

For now, you can probably revert this change in Thunderbird code. As we change a lot of the code to use the new URI API, keeping up with the changes could be a bit more difficult. But if you are sure the JS implemented URIs are only used on the main thread, you can create a new C++ wrapper that forwards calls to the JS implementation.