787981 - Use LongStringActor in the Web Console actors

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Mihai Sucan [:msucan]]

Bug 694539 landed, we now have LongStringActor support. We need to use the new actor in the Web Console, in all places fitting its use-cases.

Comment 1

[Mihai Sucan [:msucan]]

Attachment: proposed patch

This patch adds LongStringActor usage to the Web Console actors, tests included. I didn't test the LSA capabilities - we have other tests for that.

Minimal UI changes in the Web Console: for when we quote strings, I added an ellipsis: "foo"... . For cases when we don't quote strings, I didn't add anything, to avoid confusion. I don't know what kind of UX we want for requesting the rest of long strings - plus, I'd like to avoid touching the output too much - due to bug 778766. Also, the property panel will be replaced - tooltips have a limit as well: they can't even properly show the initial string.

For the network request and response bodies I did not add the LSA because what we ended up with the NetworkEventActor is something that probably does not need it: the bodies are never sent without being requested from the server specifically. When you ask for the body, you don't want only 100 bytes - you want more. The network panel has a bug we should fix - Bug 792049 - we should only display the request/response bodies on user demand. Beyond that, we should have a way to get the body in chunks of several tens/hundreds of Kb at once. Thoughts? Shall I just use LSA for the network request and response bodies? I can do it here.


Try push:
https://tbpl.mozilla.org/?tree=Try&rev=2cd91dd2018f

Comment 2

[Mozilla RelEng Bot]

Try run for 1c7fddce3438 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=1c7fddce3438
Results (out of 37 total builds):
    success: 23
    warnings: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1c7fddce3438

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 672459 [details] [diff] [review]
proposed patch

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

Good stuff!

I have some suggestions about things to improve below, but in regards to your question: I think it makes sense to just use long strings even for the network events, particularly for the headers which can be quite lengthy. But even for response bodies, why come up with a similar mechanism and not reuse the support built-in to the protocol? This way even displaying large images from a mobile device in the network panel will be less janky, since we can fetch the image piecemeal and update the panel asynchronously.

I imagine bug 792049 will be about the UI bits of fetching the request/response bodies, while the underlying transport will be based on long strings.

::: toolkit/devtools/debugger/server/dbg-script-actors.js
@@ +1646,5 @@
> +  /**
> +   * Handle a request to release this LongStringActor instance.
> +   */
> +  onRelease: function LSA_onRelease() {
> +    if (this.registeredPool) {

Actors should always have a registeredPool attached to them. If you've come across a case where it isn't, we need to fix it.

@@ +1647,5 @@
> +   * Handle a request to release this LongStringActor instance.
> +   */
> +  onRelease: function LSA_onRelease() {
> +    if (this.registeredPool) {
> +      this.registeredPool.removeActor(this.actorID);

removeActor now takes the actor object as a parameter, not the ID. Also we need to remove the entry from the actor map, like we do in LSA_disconnect.

::: toolkit/devtools/webconsole/WebConsoleUtils.jsm
@@ +592,4 @@
>        case "number":
>          return aValue;
> +      case "string":
> +          return aObjectWrapper(aValue);

You could fall through to the object/function case (removing 1 line) if you just tightened the check there, but this is a style nit, so do what you like best.

@@ +684,5 @@
>     *         The object grip converted to a string.
>     */
>    objectActorGripToString: function WCU_objectActorGripToString(aGrip, aFormatString)
>    {
>      // Primitives like strings and numbers are not sent as objects.

This comment needs updating now that strings are 'special'.

@@ +693,5 @@
> +    if (type == "string" ||
> +        (aGrip && type == "object" && aGrip.type == "longString")) {
> +      let str = type == "string" ? aGrip : aGrip.initial;
> +      let suffix = type != "string" ? "\u2026" : "";
> +      return aFormatString ? this.formatResultString(str) + suffix : str;

I think you could simplify this part by consolidating these 3 conditionals into 1 or 2 with a slight increase in line count, but I'm not going to insist on strictly style issues.

@@ +822,5 @@
>        return val;
>      }
>  
> +    if (val.type == "longString") {
> +      return this.formatResultString(val.initial) + "\u2026";

I would expect this ellipsis to be clickable so that the rest of the string would be fetched. Alternatively, if this is too much work and conflicts with the console output rewrite, I would expect that the frontend would at least fetch all of the remaining string and display it in whole. Otherwise we are regressing the functionality of the console.

Getting all of the long string may be slower than without long string actors, but it can be less janky if we yield to the event loop after each slice is retrieved.

::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
@@ +67,5 @@
>      this._isGlobalActor = true;
>    }
>  
> +  this._actorPool = new ActorPool(this.conn);
> +  this.conn.addActorPool(this._actorPool);

I like the pool unification!

@@ +231,5 @@
> +    if (typeof aObject == "string") {
> +      if (aObject.length >= DebuggerServer.LONG_STRING_LENGTH) {
> +        let actor = new LongStringActor(aObject, this);
> +        this._actorPool.addActor(actor);
> +        return actor.grip();

I see you are not using a map to avoid creating duplicate actors for the same long string, like we do in dbg-script-actors.js. Was that a conscious tradeoff because you consider the memory overhead of the extra map to exceed the cost of the duplicates?

I think that even in that case, I would rather have us be consistent and file a bug to debate removing that map from the script actors as well.

Comment 4

[Mihai Sucan [:msucan]]

Thanks for your review Panos!


(In reply to Panos Astithas [:past] from comment #3)
> Comment on attachment 672459 [details] [diff] [review]
> proposed patch
> 
> Review of attachment 672459 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Good stuff!
> 
> I have some suggestions about things to improve below, but in regards to
> your question: I think it makes sense to just use long strings even for the
> network events, particularly for the headers which can be quite lengthy. But
> even for response bodies, why come up with a similar mechanism and not reuse
> the support built-in to the protocol? This way even displaying large images
> from a mobile device in the network panel will be less janky, since we can
> fetch the image piecemeal and update the panel asynchronously.

I will update the patch to use LSA for the network event headers and for the request/response bodies. For the bodies I will have to switch to base64 encoding.


> I imagine bug 792049 will be about the UI bits of fetching the
> request/response bodies, while the underlying transport will be based on
> long strings.

Sure.

> ::: toolkit/devtools/debugger/server/dbg-script-actors.js
> @@ +1646,5 @@
> > +  /**
> > +   * Handle a request to release this LongStringActor instance.
> > +   */
> > +  onRelease: function LSA_onRelease() {
> > +    if (this.registeredPool) {
> 
> Actors should always have a registeredPool attached to them. If you've come
> across a case where it isn't, we need to fix it.

Oh, good point. I used this check simply because I saw it in other places.

> @@ +1647,5 @@
> > +   * Handle a request to release this LongStringActor instance.
> > +   */
> > +  onRelease: function LSA_onRelease() {
> > +    if (this.registeredPool) {
> > +      this.registeredPool.removeActor(this.actorID);
> 
> removeActor now takes the actor object as a parameter, not the ID. Also we
> need to remove the entry from the actor map, like we do in LSA_disconnect.

API changes ftw! Will fix. ;)


> @@ +684,5 @@
> >     *         The object grip converted to a string.
> >     */
> >    objectActorGripToString: function WCU_objectActorGripToString(aGrip, aFormatString)
> >    {
> >      // Primitives like strings and numbers are not sent as objects.
> 
> This comment needs updating now that strings are 'special'.

Yes, will fix.

> @@ +693,5 @@
> > +    if (type == "string" ||
> > +        (aGrip && type == "object" && aGrip.type == "longString")) {
> > +      let str = type == "string" ? aGrip : aGrip.initial;
> > +      let suffix = type != "string" ? "\u2026" : "";
> > +      return aFormatString ? this.formatResultString(str) + suffix : str;
> 
> I think you could simplify this part by consolidating these 3 conditionals
> into 1 or 2 with a slight increase in line count, but I'm not going to
> insist on strictly style issues.

Indeed. Will fix.

> @@ +822,5 @@
> >        return val;
> >      }
> >  
> > +    if (val.type == "longString") {
> > +      return this.formatResultString(val.initial) + "\u2026";
> 
> I would expect this ellipsis to be clickable so that the rest of the string
> would be fetched. Alternatively, if this is too much work and conflicts with
> the console output rewrite, I would expect that the frontend would at least
> fetch all of the remaining string and display it in whole. Otherwise we are
> regressing the functionality of the console.
> 
> Getting all of the long string may be slower than without long string
> actors, but it can be less janky if we yield to the event loop after each
> slice is retrieved.

Good point about the regression. To fix this issue I would propose I keep this patch with minimal UI changes. I'd like to do a part 2 patch with UI changes: clickable ellipsis so we can get the rest of the string. After that I would also like to do the network panel UI bits in bug 792049.

Does this sound fine?

I don't want to block this UI change on the output rewrite, even if it's "in the middle of the action".

> ::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
> @@ +67,5 @@
> >      this._isGlobalActor = true;
> >    }
> >  
> > +  this._actorPool = new ActorPool(this.conn);
> > +  this.conn.addActorPool(this._actorPool);
> 
> I like the pool unification!

Yes, much saner now.

> @@ +231,5 @@
> > +    if (typeof aObject == "string") {
> > +      if (aObject.length >= DebuggerServer.LONG_STRING_LENGTH) {
> > +        let actor = new LongStringActor(aObject, this);
> > +        this._actorPool.addActor(actor);
> > +        return actor.grip();
> 
> I see you are not using a map to avoid creating duplicate actors for the
> same long string, like we do in dbg-script-actors.js. Was that a conscious
> tradeoff because you consider the memory overhead of the extra map to exceed
> the cost of the duplicates?

I did this to be consistent with the web console object actors - life-time issues. If I do what the debugger does, I lose strings too soon.

Comment 5

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Mihai Sucan [:msucan] from comment #4)
> (In reply to Panos Astithas [:past] from comment #3)
> > @@ +822,5 @@
> > >        return val;
> > >      }
> > >  
> > > +    if (val.type == "longString") {
> > > +      return this.formatResultString(val.initial) + "\u2026";
> > 
> > I would expect this ellipsis to be clickable so that the rest of the string
> > would be fetched. Alternatively, if this is too much work and conflicts with
> > the console output rewrite, I would expect that the frontend would at least
> > fetch all of the remaining string and display it in whole. Otherwise we are
> > regressing the functionality of the console.
> > 
> > Getting all of the long string may be slower than without long string
> > actors, but it can be less janky if we yield to the event loop after each
> > slice is retrieved.
> 
> Good point about the regression. To fix this issue I would propose I keep
> this patch with minimal UI changes. I'd like to do a part 2 patch with UI
> changes: clickable ellipsis so we can get the rest of the string. After that
> I would also like to do the network panel UI bits in bug 792049.
> 
> Does this sound fine?
> 
> I don't want to block this UI change on the output rewrite, even if it's "in
> the middle of the action".

Sure.

> > @@ +231,5 @@
> > > +    if (typeof aObject == "string") {
> > > +      if (aObject.length >= DebuggerServer.LONG_STRING_LENGTH) {
> > > +        let actor = new LongStringActor(aObject, this);
> > > +        this._actorPool.addActor(actor);
> > > +        return actor.grip();
> > 
> > I see you are not using a map to avoid creating duplicate actors for the
> > same long string, like we do in dbg-script-actors.js. Was that a conscious
> > tradeoff because you consider the memory overhead of the extra map to exceed
> > the cost of the duplicates?
> 
> I did this to be consistent with the web console object actors - life-time
> issues. If I do what the debugger does, I lose strings too soon.

This sounds troubling, we should investigate why that happens.

Comment 6

[Mihai Sucan [:msucan]]

(In reply to Panos Astithas [:past] from comment #5)
....
> > I did this to be consistent with the web console object actors - life-time
> > issues. If I do what the debugger does, I lose strings too soon.
> 
> This sounds troubling, we should investigate why that happens.

This is a known issue and why it happens. Please see:

https://gist.github.com/3691691

...section Part 2, the introduction explains the issue.

Comment 7

[Mihai Sucan [:msucan]]

Attachment: updated patch

I am submitting the latest working patch. I still need to do the UI part for the console output and the net panel. For example the header and cookie values are not expanded to full strings.

Given the established requirements, this patch is not intended to be landable by itself - the UI work is pending.

Please let me know if I am missing something else. Thanks!

Comment 8

[Mihai Sucan [:msucan]]

Attachment: part 2: console output changes

Part 2: the output changes which add a nice [...] ellipsis. Users can click it to get the full string from the server. This patch fixes bug 584672. :)

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 677043 [details] [diff] [review]
updated patch

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

::: browser/devtools/webconsole/webconsole.js
@@ +1692,5 @@
>  
> +      let postData = aHttpActivity.request.postData;
> +      if (typeof postData.text == "object") {
> +        let longString = this.webConsoleClient.longString(postData.text);
> +        longString.substring(0, longString.length, onRequestPostDataFullString);

Here and below you are discarding the initial part that was already transferred. You could save some time by fetching from longString.initial.length up to longString.length and then concatenating the result.

I know that SC_source is equally inefficient and we should fix it, too.

A future improvement would be to fetch in chunks, so that large resources (like those in BananaBread for example) can be retrieved without UI delays.

::: toolkit/devtools/debugger/server/dbg-script-actors.js
@@ +1648,5 @@
>  
> +  /**
> +   * Handle a request to release this LongStringActor instance.
> +   */
> +  onRelease: function LSA_onRelease() {

Can you add a TODO here about adding a check for this.registeredPool === threadActor.threadLifetimePool when the web console moves away from manually releasing pause-scoped actors?

::: toolkit/devtools/webconsole/WebConsoleClient.jsm
@@ +29,5 @@
>  function WebConsoleClient(aDebuggerClient, aActor)
>  {
>    this._actor = aActor;
>    this._client = aDebuggerClient;
> +  this._grips = {};

If all you store here are long strings and not other kinds of actor grips, then perhaps it would be best to name it accordingly (_longStrings or similar).

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 677554 [details] [diff] [review]
part 2: console output changes

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

It seems like WCU_getObjectGrip should also use long strings for displayString. I tried the test case in bug 584672 and I got the whole string in the response. Also, don't you want to add the ellipsis to the network panel as well? Headers and bodies are displayed in full right now and it could be slow for the remote cases.

::: browser/devtools/webconsole/webconsole.js
@@ +2460,5 @@
> +      aFormatter = function(s) s;
> +    }
> +
> +    let longString = this.webConsoleClient.longString(aActor);
> +    longString.substring(0, longString.length,

Here, too, you can request from longString.initial.length onwards.

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

One more thing: https://developer.mozilla.org/en-US/docs/DOM/window.btoa#Unicode_Strings

Did you try using this patch with google.jp or something?

Comment 12

[Mihai Sucan [:msucan]]

(In reply to Panos Astithas [:past] from comment #9)
> Comment on attachment 677043 [details] [diff] [review]
> updated patch
> 
> Review of attachment 677043 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/webconsole/webconsole.js
> @@ +1692,5 @@
> >  
> > +      let postData = aHttpActivity.request.postData;
> > +      if (typeof postData.text == "object") {
> > +        let longString = this.webConsoleClient.longString(postData.text);
> > +        longString.substring(0, longString.length, onRequestPostDataFullString);
> 
> Here and below you are discarding the initial part that was already
> transferred. You could save some time by fetching from
> longString.initial.length up to longString.length and then concatenating the
> result.
> 
> I know that SC_source is equally inefficient and we should fix it, too.

Will fix. I just did it like this because the initial string is only 100 chars - not a network problem, but I can see your point.


> ::: toolkit/devtools/debugger/server/dbg-script-actors.js
> @@ +1648,5 @@
> >  
> > +  /**
> > +   * Handle a request to release this LongStringActor instance.
> > +   */
> > +  onRelease: function LSA_onRelease() {
> 
> Can you add a TODO here about adding a check for this.registeredPool ===
> threadActor.threadLifetimePool when the web console moves away from manually
> releasing pause-scoped actors?

Will do.

> ::: toolkit/devtools/webconsole/WebConsoleClient.jsm
> @@ +29,5 @@
> >  function WebConsoleClient(aDebuggerClient, aActor)
> >  {
> >    this._actor = aActor;
> >    this._client = aDebuggerClient;
> > +  this._grips = {};
> 
> If all you store here are long strings and not other kinds of actor grips,
> then perhaps it would be best to name it accordingly (_longStrings or
> similar).

Sure.

Thanks for the review!

Comment 13

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Mihai Sucan [:msucan] from comment #12)
> (In reply to Panos Astithas [:past] from comment #9)
> > ::: browser/devtools/webconsole/webconsole.js
> > @@ +1692,5 @@
> > >  
> > > +      let postData = aHttpActivity.request.postData;
> > > +      if (typeof postData.text == "object") {
> > > +        let longString = this.webConsoleClient.longString(postData.text);
> > > +        longString.substring(0, longString.length, onRequestPostDataFullString);
> > 
> > Here and below you are discarding the initial part that was already
> > transferred. You could save some time by fetching from
> > longString.initial.length up to longString.length and then concatenating the
> > result.
> > 
> > I know that SC_source is equally inefficient and we should fix it, too.
> 
> Will fix. I just did it like this because the initial string is only 100
> chars - not a network problem, but I can see your point.

Thanks. Note that LONG_STRING_INITIAL_LENGTH is actually 1000 chars and there is still room for tuning those parameters.

Comment 14

[Mihai Sucan [:msucan]]

Yes, 1000 chars, bad typo in my comment.


(In reply to Panos Astithas [:past] from comment #10)
> Comment on attachment 677554 [details] [diff] [review]
> part 2: console output changes
> 
> Review of attachment 677554 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It seems like WCU_getObjectGrip should also use long strings for
> displayString. I tried the test case in bug 584672 and I got the whole
> string in the response.

This is in my TODO.

> Also, don't you want to add the ellipsis to the
> network panel as well? Headers and bodies are displayed in full right now
> and it could be slow for the remote cases.

Will do this in part 3.

> ::: browser/devtools/webconsole/webconsole.js
> @@ +2460,5 @@
> > +      aFormatter = function(s) s;
> > +    }
> > +
> > +    let longString = this.webConsoleClient.longString(aActor);
> > +    longString.substring(0, longString.length,
> 
> Here, too, you can request from longString.initial.length onwards.

Will fix.

Thanks!

Comment 15

[Mihai Sucan [:msucan]]

Attachment: part 1: web console actors changes

Addressed review comments.

Comment 16

[Mihai Sucan [:msucan]]

Attachment: part 2: console output changes

Addressed review comments. Thank you Panos!

A note on Unicode strings: google.jp works fine. Response body is displayed correctly. However, do note base64 encoding is used only for non-text MIME types. Those MIME types are not displayed in the network panel.

In part 3 patch I will make the netpanel changes and I intend to use an <iframe type=content> to put the data URI into, such that the actual image received from the server is displayed - see bug 792043.

Please let me know if you have any further concerns with these two patches.

Comment 17

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Mihai Sucan [:msucan] from comment #16)
> A note on Unicode strings: google.jp works fine. Response body is displayed
> correctly. However, do note base64 encoding is used only for non-text MIME
> types. Those MIME types are not displayed in the network panel.

Ah, I missed that. 

> In part 3 patch I will make the netpanel changes and I intend to use an
> <iframe type=content> to put the data URI into, such that the actual image
> received from the server is displayed - see bug 792043.

Sounds great.

Comment 18

[Mihai Sucan [:msucan]]

Attachment: part 3: network panel updates

This patch adds an ellipsis to long values for headers and cookies. I also add a new display for received cookies from the server (we were displaying those in the response headers view).

I also made the net panel to not automatically fetch the complete response body - you can manually request that. We might want to do this automatically for local connections. Thoughts?

This patch should fix bugs 621291, 792043 and 792049 as well.

Please let me know if I missed anything. I'd like to add more testing before pushing it. Thanks!

Comment 19

[Mihai Sucan [:msucan]]

Attachment: part 3: network panel changes

More test code and a fix for the request body handling in the netpanel jsm.

Comment 20

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 679739 [details] [diff] [review]
part 3: network panel changes

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

::: browser/locales/en-US/chrome/browser/devtools/webConsole.dtd
@@ +11,5 @@
>  <!ENTITY window.title "Web Console">
>  
> +<!ENTITY networkPanel.requestURL                  "Request URL:">
> +<!ENTITY networkPanel.requestMethod               "Request Method:">
> +<!ENTITY networkPanel.statusCode                  "Status Code:">

Don't you have to change the label when modifying the string for DTDs?

::: browser/locales/en-US/chrome/browser/devtools/webconsole.properties
@@ +182,5 @@
> +
> +# LOCALIZATION NOTE (NetworkPanel.fetchRemainingRequestContentLink): This is
> +# displayed in the network panel when the request body is only partially
> +# available.
> +NetworkPanel.fetchRemainingRequestContentLink=Fetch the request body (%1$S bytes).

I think we don't usually add full stops in actionable content (links, buttons). The bold text with a full stop may not immediately look something that a user can click. Maybe adding an underline or similar would help?

::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
@@ +279,4 @@
>     */
>    releaseActor: function WCA_releaseActor(aActor)
>    {
> +    let id = typeof aActor == "string" ? aActor : aActor.actorID;

What are the cases where the caller doesn't have a reference to the actor?

Comment 21

[Mihai Sucan [:msucan]]

(In reply to Panos Astithas [:past] from comment #20)
> Comment on attachment 679739 [details] [diff] [review]
> part 3: network panel changes
> 
> Review of attachment 679739 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/locales/en-US/chrome/browser/devtools/webConsole.dtd
> @@ +11,5 @@
> >  <!ENTITY window.title "Web Console">
> >  
> > +<!ENTITY networkPanel.requestURL                  "Request URL:">
> > +<!ENTITY networkPanel.requestMethod               "Request Method:">
> > +<!ENTITY networkPanel.statusCode                  "Status Code:">
> 
> Don't you have to change the label when modifying the string for DTDs?

Will do.


> ::: browser/locales/en-US/chrome/browser/devtools/webconsole.properties
> @@ +182,5 @@
> > +
> > +# LOCALIZATION NOTE (NetworkPanel.fetchRemainingRequestContentLink): This is
> > +# displayed in the network panel when the request body is only partially
> > +# available.
> > +NetworkPanel.fetchRemainingRequestContentLink=Fetch the request body (%1$S bytes).
> 
> I think we don't usually add full stops in actionable content (links,
> buttons). The bold text with a full stop may not immediately look something
> that a user can click. Maybe adding an underline or similar would help?

Will add an underline.

> ::: toolkit/devtools/webconsole/dbg-webconsole-actors.js
> @@ +279,4 @@
> >     */
> >    releaseActor: function WCA_releaseActor(aActor)
> >    {
> > +    let id = typeof aActor == "string" ? aActor : aActor.actorID;
> 
> What are the cases where the caller doesn't have a reference to the actor?

Yes. See NEA_release(): I only keep the LSA grips, not the actor objects themselves. I can avoid doing this change - makes sense. Will fix.

Thank you for the r+!

Comment 22

[Mihai Sucan [:msucan]]

Attachment: part 3: network panel changes

Addressed the review comments.

I will land these patches once I get a green try push.

Comment 23

[Mihai Sucan [:msucan]]

Attachment: folded patch

Folded patch, with minor test fixes after try runs.

Landed:
https://hg.mozilla.org/integration/fx-team/rev/064a5de31168

Thank you Panos!

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/064a5de31168

Comment 25

[Mozilla RelEng Bot]

Try run for 5123cefc773c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5123cefc773c
Results (out of 53 total builds):
    success: 46
    warnings: 6
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-5123cefc773c

Comment 26

[Mozilla RelEng Bot]

Try run for 25b0b01f1ff7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=25b0b01f1ff7
Results (out of 52 total builds):
    success: 39
    warnings: 12
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-25b0b01f1ff7