1165162 - Include all non-default app attributes in stringified nsIPrincipal::origin

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Bobby Holley (:bholley)]

Splitting this out as discussed in bug 1163254 comment 7. This is actually the real meat of the changes, in that it creates the generalized framework by which we can easily add other app attributes.

Yoshi should be able to build on this once it's green.

Comment 1

[Bobby Holley (:bholley)]

Given the behavior change for .origin, this may go orange:

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

Comment 2

[Bobby Holley (:bholley)]

Alright, so this should be relatively code-complete, modulo the various failures I need to look at the result from my munging of .origin. I'm out of time to look at those today, and will have a look on Monday.

Meanwhile, Jonas - if you have a few minutes to give any brief feedback you'd like to make on the API, this would be a good time to do it (i.e. before I go around fixing up various consumers).

You can scrape the patches here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=689a8a0292c8

I'll also upload a zip of patches in sequence that you can qimport if you prefer.

Comment 3

[Bobby Holley (:bholley)]

Attachment: patches.zip

Comment 4

[Bobby Holley (:bholley)]

I just looked at the test_defaultStorageUpgrade.js failure. The basic problem is that it's using a packaged profile, and so it doesn't find the IDB database it expects ("1007+f+app+++system.gaiamobile.org") because it's now called "1007+f+app+++system.gaiamobile.org+appId=1007".

We can obviously fix up the packaged profile, but we should think about what kind if migration scheme, if any, we'll need here.

Comment 5

[Bobby Holley (:bholley)]

Fixed a bug in GetAppStatusForPrincipal. This should be greener:

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

Comment 6

[Bobby Holley (:bholley)]

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

Comment 7

[Bobby Holley (:bholley)]

Attachment: Part 1 - Make OriginAttributes a dictionary, and make it accessible as both a jsval and a canonical string. v1

Comment 8

[Bobby Holley (:bholley)]

Attachment: Part 2 - Rework the nsIScriptSecurityManager principal-minting API to be originAttributes-centric. v1

Comment 9

[Bobby Holley (:bholley)]

Attachment: Part 3 - Fix up nsScriptSecurityManager::AppStatusForPrincipal to compare principals rather than origins. v1

The current check will fail once we start munging the format of nsIPrincipal::Origin.

Comment 10

[Bobby Holley (:bholley)]

Attachment: Part 4 - Hoist GetOrigin onto BasePrincipal. v1

Comment 11

[Bobby Holley (:bholley)]

Attachment: Part 5 - Serialize originSuffix into .origin. v2

We also provide an opt-out for the original behavior, and use it in various
consumers that look like they need fixing up. Most of the usage here is in
code with persistence considerations, where we may need some sort of migration
path.

Comment 12

[Bobby Holley (:bholley)]

Attachment: Part 6 - Tests. v1

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8607138 [details] [diff] [review]
Part 1 - Make OriginAttributes a dictionary, and make it accessible as both a jsval and a canonical string. v1

Review of attachment 8607138 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/nsIPrincipal.idl
@@ +174,5 @@
> +    [implicit_jscontext]
> +    readonly attribute jsval originAttributes;
> +
> +    /**
> +     * A string of the form ?key1=value1&key2=value2, where each pair represents

I feel like there are more benefits than downsides restrict ourselves to characters that can be used in filenames here. Which I think basically means switching the '?' to something else.

Might even be worth simply using '&' as to not have to use different separator for the first key/value pair.

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8607140 [details] [diff] [review]
Part 2 - Rework the nsIScriptSecurityManager principal-minting API to be originAttributes-centric. v1

Review of attachment 8607140 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/nsIScriptSecurityManager.idl
@@ +191,5 @@
> +     * See nsIPrincipal.h for a description of origin attributes, and
> +     * SystemDictionaries.webidl for a list of origin attributes and their defaults.
> +     */
> +    [implicit_jscontext]
> +    nsIPrincipal createNullPrincipal(in jsval originAttributes);

Does it really make sense to set originAttributes on null principals?

A null principal should never be permitted to store any data to disk, and doesn't appear to be allowed to make network requests.

I'm not necessarily opposed, more curious why.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8607141 [details] [diff] [review]
Part 3 - Fix up nsScriptSecurityManager::AppStatusForPrincipal to compare principals rather than origins. v1

Review of attachment 8607141 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/nsScriptSecurityManager.cpp
@@ +290,5 @@
>  
> +    // The app could contain a cross-origin iframe - make sure that the content
> +    // is actually same-origin with the app.
> +    nsCOMPtr<nsIPrincipal> appPrin;
> +    OriginAttributes attrs(aPrin->GetAppId(), false);

might as well use |appId| here. And might be worth commenting on why you are using 'false' rather than 'inMozBrowser' as second argument.

Hmm... Though shouldn't it be possible to get an OriginAttributes off of the nsIPrincipal? I thought the point was that we wanted code like this be able to just grab the OriginAttributes and forward that as an opaque object?

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8607143 [details] [diff] [review]
Part 5 - Serialize originSuffix into .origin. v2

Review of attachment 8607143 [details] [diff] [review]:
-----------------------------------------------------------------

Is the plan here to do a followup bug to change a bunch of these from .originNoSuffix to use .origin?

sr=me assuming that's the case.

Comment 17

[Bobby Holley (:bholley)]

(In reply to Jonas Sicking (:sicking) from comment #13)
> I feel like there are more benefits than downsides restrict ourselves to
> characters that can be used in filenames here. Which I think basically means
> switching the '?' to something else.
> 
> Might even be worth simply using '&' as to not have to use different
> separator for the first key/value pair.

Ok - I replaced it with |!|, which I think is a bit more recognizable and is easier to swap out with my existing JS parsing code.

(In reply to Jonas Sicking (:sicking) from comment #14)
> Does it really make sense to set originAttributes on null principals?
> 
> A null principal should never be permitted to store any data to disk, and
> doesn't appear to be allowed to make network requests.
> 
> I'm not necessarily opposed, more curious why.

I'm not sure actually, but we currently have code that explicitly handles this, so I'm assuming that there's some FirefoxOS-y reason. I added the SSM API so that I could properly test the code that we have.

(In reply to Jonas Sicking (:sicking) from comment #15)
> might as well use |appId| here. And might be worth commenting on why you are
> using 'false' rather than 'inMozBrowser' as second argument.

Ok.

> Hmm... Though shouldn't it be possible to get an OriginAttributes off of the
> nsIPrincipal? I thought the point was that we wanted code like this be able
> to just grab the OriginAttributes and forward that as an opaque object?

Which nsIPrincipal? We're comparing an nsIPrincipal with a mozIApplication here. It would be nice if mozIApplication exposed a principal instead of an origin string, but that isn't the case right now, so we need to synthesize one.

(In reply to Jonas Sicking (:sicking) from comment #16)
> Is the plan here to do a followup bug to change a bunch of these from
> .originNoSuffix to use .origin?

Yes, these are basically breadcrumbs for Yoshi to go fix up. :-)

Comment 18

[Bobby Holley (:bholley)]

Attachment: Part 7 - Add nsIPrincipal::cookieJar. v1

Per today's discussion.

Comment 19

[Bobby Holley (:bholley)]

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

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8607335 [details] [diff] [review]
Part 7 - Add nsIPrincipal::cookieJar. v1

Review of attachment 8607335 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/BasePrincipal.cpp
@@ +160,5 @@
> +BasePrincipal::GetCookieJar(nsACString& aCookieJar)
> +{
> +  // We just forward to .jarPrefix for now, which is a nice compact
> +  // stringification of the (appId, inBrowser) tuple. This will eventaully be
> +  // swapped out for an origin attribute - see the comment in nsIPrincipal.idl.

"origin attribute" sounds wrong here, since "cookie jar" is exactly the part of .origin which isn't really an "origin".

Maybe "generic attribute" instead? Or "generic mechanism"?

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to Bobby Holley (:bholley) from comment #17)
> > Hmm... Though shouldn't it be possible to get an OriginAttributes off of the
> > nsIPrincipal? I thought the point was that we wanted code like this be able
> > to just grab the OriginAttributes and forward that as an opaque object?
> 
> Which nsIPrincipal? We're comparing an nsIPrincipal with a mozIApplication
> here. It would be nice if mozIApplication exposed a principal instead of an
> origin string, but that isn't the case right now, so we need to synthesize
> one.

You're lifting the appid+inbrowser from aPrincipal. I.e. you're lifting all the "origin suffix" from aPrincipal. Except that the test higher up ensures that inbrowser==false by the time we get down here.

What this code is actually doing, is to do a same-origin check between the aPrincipal.uri and the mozIApplication.manifestURL. But it does so by creating a new nsIPrincipal where all the properties except the uri is copied from aPrincipal.

Comment 22

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8607138 [details] [diff] [review]
Part 1 - Make OriginAttributes a dictionary, and make it accessible as both a jsval and a canonical string. v1

Review of attachment 8607138 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/BasePrincipal.h
@@ +56,5 @@
> +    OriginAttributes() {}
> +    OriginAttributes(uint32_t aAppId, bool aInBrowser)
> +    {
> +      mAppId = aAppId;
> +      mInBrowser = aInBrowser;

why did you change the usual : mAppId(aAppId) syntax here?

::: dom/webidl/SystemDictionaries.webidl
@@ +17,5 @@
> + * methods.
> + */
> +dictionary OriginAttributesDictionary {
> +  unsigned long appId = 0;
> +  boolean inBrowser = false;

Why do we call it inBrowser here instead of the new inBrowserElement you started to use lately?

Comment 23

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8607140 [details] [diff] [review]
Part 2 - Rework the nsIScriptSecurityManager principal-minting API to be originAttributes-centric. v1

Review of attachment 8607140 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know about this. I see it's deprecated but I get 234 matches for getCodebasePrincipal in add-on code... Why don't we just make the second argument optional and default it to {} ?

Comment 24

[Bobby Holley (:bholley)]

(In reply to Jonas Sicking (:sicking) from comment #20)
> "origin attribute" sounds wrong here, since "cookie jar" is exactly the part
> of .origin which isn't really an "origin".

It exactly fits the definition of "origin attribute" AFAICT - it's an attribute with its own semantic meaning which must be compared (along with other origin attributes) when computing same-origin-ness.
 
> Maybe "generic attribute" instead? Or "generic mechanism"?

My understand of our agreement from yesterday is that, long-term, the cookie jar would probably map exactly to a value called userContext or somesuch, which would need to live in originAttributes per the first paragraph above. We're introducing this getter so that we can seamlessly switch to it from the current appId:inBrowser scheme we have now.

So the comment seems entirely accurate to me. Am I miss-remembering something?

(In reply to Jonas Sicking (:sicking) from comment #21)
> You're lifting the appid+inbrowser from aPrincipal. I.e. you're lifting all
> the "origin suffix" from aPrincipal. Except that the test higher up ensures
> that inbrowser==false by the time we get down here.
> 
> What this code is actually doing, is to do a same-origin check between the
> aPrincipal.uri and the mozIApplication.manifestURL. But it does so by
> creating a new nsIPrincipal where all the properties except the uri is
> copied from aPrincipal.

The way I see it, this code is basically doing a principal equality check between aPrincipal and a virtual principal (with an origin, an appId, and inBrowser=false) that is deducible from mozIApplication (and would ideally be exposed by a getter on mozIApplication). It's not clear to me what semantics we want here with respect to other origin attributes, which is why I did things explicitly for now.

Comment 25

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #22)
> ::: caps/BasePrincipal.h
> @@ +56,5 @@
> > +    OriginAttributes() {}
> > +    OriginAttributes(uint32_t aAppId, bool aInBrowser)
> > +    {
> > +      mAppId = aAppId;
> > +      mInBrowser = aInBrowser;
> 
> why did you change the usual : mAppId(aAppId) syntax here?

To make it clear that these members exist in and are initialized by the superclass.

> ::: dom/webidl/SystemDictionaries.webidl
> @@ +17,5 @@
> > + * methods.
> > + */
> > +dictionary OriginAttributesDictionary {
> > +  unsigned long appId = 0;
> > +  boolean inBrowser = false;
> 
> Why do we call it inBrowser here instead of the new inBrowserElement you
> started to use lately?

Jonas is concerned about character length when serializing to filenames and whatnot. I'm happy to use aInBrowser everywhere going forward.

Comment 26

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to Bobby Holley (:bholley) from comment #24)
> (In reply to Jonas Sicking (:sicking) from comment #20)
> > "origin attribute" sounds wrong here, since "cookie jar" is exactly the part
> > of .origin which isn't really an "origin".
> 
> It exactly fits the definition of "origin attribute" AFAICT - it's an
> attribute with its own semantic meaning which must be compared (along with
> other origin attributes) when computing same-origin-ness.

I don't feel strongly.

> (In reply to Jonas Sicking (:sicking) from comment #21)
> > You're lifting the appid+inbrowser from aPrincipal. I.e. you're lifting all
> > the "origin suffix" from aPrincipal. Except that the test higher up ensures
> > that inbrowser==false by the time we get down here.
> > 
> > What this code is actually doing, is to do a same-origin check between the
> > aPrincipal.uri and the mozIApplication.manifestURL. But it does so by
> > creating a new nsIPrincipal where all the properties except the uri is
> > copied from aPrincipal.
> 
> The way I see it, this code is basically doing a principal equality check
> between aPrincipal and a virtual principal (with an origin, an appId, and
> inBrowser=false) that is deducible from mozIApplication (and would ideally
> be exposed by a getter on mozIApplication). It's not clear to me what
> semantics we want here with respect to other origin attributes, which is why
> I did things explicitly for now.

What I'm saying above is the semantic meaning that this code wants to have in a generic sense. I.e. it will want to copy all parts of OriginAttributes over and only change the URI.

Comment 27

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

> > why did you change the usual : mAppId(aAppId) syntax here?
> 
> To make it clear that these members exist in and are initialized by the
> superclass.

C++ doesn't allow using the initalizer-list to set members that live in the superclass. So :mAppId(aAppId) wouldn't compile.

Comment 28

[Bobby Holley (:bholley)]

(In reply to Jonas Sicking (:sicking) from comment #26)
> What I'm saying above is the semantic meaning that this code wants to have
> in a generic sense. I.e. it will want to copy all parts of OriginAttributes
> over and only change the URI.

Is it, though? So far, it's 50-50 in terms of which of the origin attributes it wants to copy (i.e. it explicitly doesn't want to copy mozBrowser - we could even nix the inBrowser bailout case at the beginning and just rely on the principals not comparing equal).

I understand that the author originally wrote the code with the more humble intent of "just comparing origins", but I think that "compare the principal of the code with the (virtual) principal of the app in the registry" is a more useful and robust conceptual model.

I don't feel strongly about it, I'm just not convinced that copying all origin attributes is clearly better. I'd be fine to switch the patch to doing that if that's what you'd prefer though.

Comment 29

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #23)
> I don't know about this. I see it's deprecated but I get 234 matches for
> getCodebasePrincipal in add-on code...

Oh, yuck.

> Why don't we just make the second argument optional and default it to {} ?

I want callers to be explicit about it, otherwise most people will probably never realize that there's a second argument.

I'm going to just introduce a new method called |createCodebasePrincipal| and deprecate the others.

Comment 30

[Bobby Holley (:bholley)]

Attachment: Part 2 - Rework the nsIScriptSecurityManager principal-minting API to be originAttributes-centric. v3 sr=sicking

Comment 31

[Bobby Holley (:bholley)]

Attachment: Part 6 - Tests. v3

Comment 32

[Bobby Holley (:bholley)]

Attachment: Part 0.0 - Make requestsync test failures more useful. v1

Comment 33

[Bobby Holley (:bholley)]

Attachment: Part 0.5 - Stop recreating principals from the message manager. v1

Comment 34

[Bobby Holley (:bholley)]

Comment on attachment 8608203 [details] [diff] [review]
Part 0.5 - Stop recreating principals from the message manager. v1

It's not clear to me why these callers are doing it this way - Jonas, do you have any idea?

Comment 35

[Bobby Holley (:bholley)]

Attachment: Part 8 - Introduce a helper for converting from origin strings to a principal. v1

I didn't end up needing this in bug, but I think it's handy to have around.

Comment 36

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8607833 [details] [diff] [review]
Part 2 - Rework the nsIScriptSecurityManager principal-minting API to be originAttributes-centric. v3 sr=sicking

Review of attachment 8607833 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/BasePrincipal.cpp
@@ +203,5 @@
> +    uriPrinc->GetPrincipal(getter_AddRefs(principal));
> +    if (!principal) {
> +      return nsNullPrincipal::Create();
> +    }
> +    nsRefPtr<BasePrincipal> concrete = dont_AddRef(Cast(principal.forget().take()));

Why are you trying to avoid addreffing and releasing here? Can you just get rid of the forget and dont_AddRef here to make the code simpler?

Comment 37

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #36)
> Why are you trying to avoid addreffing and releasing here? Can you just get
> rid of the forget and dont_AddRef here to make the code simpler?

Just trying to avoid unnecessary calls to addref/release, but I can buy that it's more trouble than it's worth here. I'll simplify it.

Comment 38

[Gabor Krizsanits (INACTIVE)]

Comment on attachment 8607141 [details] [diff] [review]
Part 3 - Fix up nsScriptSecurityManager::AppStatusForPrincipal to compare principals rather than origins. v1

Review of attachment 8607141 [details] [diff] [review]:
-----------------------------------------------------------------

Now I can totally see why you were against putting important logic inside NS_ENSURE_SUCCESS macros... looks all correct though :)

Comment 39

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8608203 [details] [diff] [review]
Part 0.5 - Stop recreating principals from the message manager. v1

Related bugs 
https://bugzilla.mozilla.org/show_bug.cgi?id=916091
https://bugzilla.mozilla.org/show_bug.cgi?id=1030997

As far as I see, we do the same thing on message manager side.

Comment 40

[Bobby Holley (:bholley)]

Attachment: Part 0.1 - Fix up test_manager.js to use real principals. v1

Comment 41

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8607141 [details] [diff] [review]
Part 3 - Fix up nsScriptSecurityManager::AppStatusForPrincipal to compare principals rather than origins. v1

Review of attachment 8607141 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, lets leave this as-is.

Comment 42

[Bobby Holley (:bholley)]

Comment on attachment 8608202 [details] [diff] [review]
Part 0.0 - Make requestsync test failures more useful. v1

Switching this to sicking, since I'll bet baku's gone to bed.

Comment 43

[Bobby Holley (:bholley)]

Comment on attachment 8608202 [details] [diff] [review]
Part 0.0 - Make requestsync test failures more useful. v1

Ok, sicking needs to go, so I'm just going to flag various people for retroactive review on these bits and push it to inbound.

Comment 44

[Bobby Holley (:bholley)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c2cd9f48962
https://treeherder.mozilla.org/#/jobs?repo=try&revision=22a99e7eeab9

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=eabe83ede1e3

Comment 45

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c081648c06e3
https://hg.mozilla.org/integration/mozilla-inbound/rev/a005f1452f41
https://hg.mozilla.org/integration/mozilla-inbound/rev/121b818c01a5
https://hg.mozilla.org/integration/mozilla-inbound/rev/72f81e3ed9aa
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc2395aa710
https://hg.mozilla.org/integration/mozilla-inbound/rev/61aa80fb79b6
https://hg.mozilla.org/integration/mozilla-inbound/rev/67d449223191
https://hg.mozilla.org/integration/mozilla-inbound/rev/85402fd3e3b4
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb2c0a02352e
https://hg.mozilla.org/integration/mozilla-inbound/rev/e44ba0c0744a

Comment 47

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8607143 [details] [diff] [review]
Part 5 - Serialize originSuffix into .origin. v2

Review of attachment 8607143 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/bluetooth2/BluetoothAdapter.cpp
@@ +982,5 @@
>    uint16_t appStatus = nsIPrincipal::APP_STATUS_NOT_INSTALLED;
>    nsAutoCString appOrigin;
>  
>    doc->NodePrincipal()->GetAppStatus(&appStatus);
> +  doc->NodePrincipal()->GetOriginNoSuffix(getter_Copies(appOrigin));

need to rebase this for Bug 1165835 which removed getter_Copies

Comment 48

[Bobby Holley (:bholley)]

(In reply to Yoshi Huang[:allstars.chh] from comment #47)
> Comment on attachment 8607143 [details] [diff] [review]
> Part 5 - Serialize originSuffix into .origin. v2
> 
> Review of attachment 8607143 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/bluetooth/bluetooth2/BluetoothAdapter.cpp
> @@ +982,5 @@
> >    uint16_t appStatus = nsIPrincipal::APP_STATUS_NOT_INSTALLED;
> >    nsAutoCString appOrigin;
> >  
> >    doc->NodePrincipal()->GetAppStatus(&appStatus);
> > +  doc->NodePrincipal()->GetOriginNoSuffix(getter_Copies(appOrigin));
> 
> need to rebase this for Bug 1165835 which removed getter_Copies

Yes, included in the version pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/85402fd3e3b4

Comment 49

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 8608205 [details] [diff] [review]
Part 8 - Introduce a helper for converting from origin strings to a principal. v1

Review of attachment 8608205 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/modules/BrowserUtils.jsm
@@ +94,5 @@
>      return Services.io.newURI(aCPOWURI.spec, aCPOWURI.originCharset, null);
>    },
>  
> +  // Creates a codebase principal from a canonical origin string. This is the
> +  // the inverse operation of .origin on a codebase principal.

remove 'the', you already had it from last line.

Comment 50

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c081648c06e3
https://hg.mozilla.org/mozilla-central/rev/a005f1452f41
https://hg.mozilla.org/mozilla-central/rev/121b818c01a5
https://hg.mozilla.org/mozilla-central/rev/72f81e3ed9aa
https://hg.mozilla.org/mozilla-central/rev/5bc2395aa710
https://hg.mozilla.org/mozilla-central/rev/61aa80fb79b6
https://hg.mozilla.org/mozilla-central/rev/67d449223191
https://hg.mozilla.org/mozilla-central/rev/85402fd3e3b4
https://hg.mozilla.org/mozilla-central/rev/cb2c0a02352e
https://hg.mozilla.org/mozilla-central/rev/e44ba0c0744a
https://hg.mozilla.org/mozilla-central/rev/eabe83ede1e3

Comment 51

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6529d63e0a43

Comment 52

[Ekanan Ketunuti]

https://hg.mozilla.org/mozilla-central/rev/6529d63e0a43