1201949 - Construct MemoryTool controller with fluxify

Status: RESOLVED FIXED

(DevTools :: Memory, defect)

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201949-memory-fluxify.patch

First crack at this that's working -- thoughts on this structure? Have seen a few different ways...

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Oh this has Redux in there -- wanted to use that since I could use docs while using it, but if Fluxify can change the method names, or I guess that's not necessary even, will remove Redux then (and won't land it regardless)

Comment 3

[James Long (:jlongster)]

Comment on attachment 8658273 [details] [diff] [review]
1201949-memory-fluxify.patch

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

I'll file bugs to normalize fluxify's API to redux's. Should just be changing "dispatcher" to "store" and "stores" to "reducers". It probably would be good to hold off landing redux for now so we are building on the exact same stuff. It's easy to replicate (the lib is so small) and we can add the `emitChange` stuff so it works better with non-React views for now.

I think the only thing `fluxify` doesn't have is a generic "subscribe" method, but I can add that. haven't needed it because our UI doesn't work with arbitrary state changes, it works with specific change events from `emitChange`. In fact, I don't see any UI hooked up here, is that right?

::: browser/devtools/memory/actions/snapshot.js
@@ +2,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +"use strict";
> +
> +const { ACTIONS } = require("../constants");

Personally I don't think you need to capitalize this. You're in a module, so it's not global, it's just "top-level". I also think you need to call this "constants" and just do `const constants = require("../constants")`

::: browser/devtools/memory/constants.js
@@ +17,5 @@
> +// Fired by Front when returning a snapshot.
> +ACTIONS.RECEIVE_SNAPSHOT = "receive-snapshot";
> +
> +// Fired by Front when returning a snapshot if it fails.
> +ACTIONS.RECEIVE_SNAPSHOT_ERROR = "receive-snapshot-error";

If you shape the async action creator as I talked about in the other comment, all you'll need to do is `exports.TAKE_SNAPSHOT = "TAKE_SNAPSHOT"`. It's also good to go ahead and capitalize the string name too so it's consistent.

::: browser/devtools/memory/controller.js
@@ +20,3 @@
>  
> +FLUXIFY_METHODS_TO_PIPE.map(m =>
> +  MemoryController.prototype[m] = function (...args) { return this.store[m](...args); });

Interesting. So `MemoryController` is basically your single global object. That makes sense for us now, but note that a neat thing about React is that components have something called a "context", and you can render a whole tree with a context and all children (with a semi-special API) can grab stuff from the context object. The react-render project uses this to render everything with a "store" in the context, so that's how components get the ability to dispatch actions through the store.

This is a cool idea in this context though.

::: browser/devtools/memory/memory.xhtml
@@ +15,1 @@
>      <link rel="stylesheet" href="chrome://browser/content/devtools/widgets.css" type="text/css"/>

Why do you use XHTML instead of HTML5? Curious because I've seen this elsewhere too. Do we have components that require this?

(comment not applicable to this line, this file existed before)

::: browser/devtools/memory/subscribers/front.js
@@ +14,5 @@
> +  this._unsubscribe();
> +  this.store = this.front = null;
> +};
> +
> +Front.prototype._listener = function () {

I don't think this is the best way to do this workflow. You're listening to a state change only to perform a non-UI related action, but the only things that should really listen to state changes is the UI. You want to keep the flow "forward" as much as possible: actions -> state change -> UI update. There are definitely exceptions, but most things work well that way.

That way, when looking at a log of actions, you know that they all came from the UI, which helps a bit if trying to figure out where they came from.

What you want is an asynchronous action creator: https://rackt.github.io/redux/docs/advanced/AsyncActions.html#async-action-creators. You could say that this `_listener` function is one, but this code should just be directly in the `takeSnapshot` action creator in `actions/snapshot.js`. In fact it collapses all of your action creators into a single one:

function takeSnapshot() {
  const id = SNAPSHOT_ID_INC++;

  return dispatch => {
    // This doesn't have to be `MemoryController.front`, but you need
    // some global API object to actually perform async work. Maybe
    // you were trying to avoid that which is why you listened to
    // state changes. If you don't want to have this global reference,
    // we could create another middleware that allows us to return a
    // function that accepts an "api" object instead of a "dispatch"
    // function.
    MemoryController.front.saveHeapSnapshot().then(platformID => {
      dispatch({
        type: ACTIONS.RECEIVE_SNAPSHOT,
        platformID: platformID,
        id: id
      })
    }, error => {
      dispatch({
        type: ACTIONS.RECEIVE_SNAPSHOT_ERROR,
        error: error,
        id: id
      });
    });
  }
}

That's a lot of code though for emitting success/error actions (I didn't even include emitting a "starting" action before `saveHeapSnapshot` is called so you can do loading screens or optimistic updates), and you're manually creating the request ID. We can wrap all this work up into a middleware, which is what my promise middleware does: https://github.com/jlongster/gecko-dev/blob/debugger-refactor-sources2/browser/devtools/shared/fluxify/promiseMiddleware.js. If you add this middleware, you can just have this:


const { PROMISE } = require('devtools/shared/fluxify/promiseMiddleware');

function takeSnapshot() {
  return {
    type: ACTIONS.TAKE_SNAPSHOT
    [PROMISE]: MemoryController.front.saveHeapSnapshot()
  };
}

It will emit actions for starting, finishing, and erroring a promise, and automatically generate a sequence ID. It also allows you to specify a HISTOGRAM_ID if you want to automatically submit timings to it.

The actions it emits looks like `{ type: ACTIONS.TAKE_SNAPSHOT, status: 'start', seqId: <id>}` and `{ type: ACTIONS.TAKE_SNAPSHOT, status: 'done', value: {}, seqId: <id>}` for starting/finishing the request. I'm totally open to changing the names of these fields, but we should standardize what async actions look like so we can generalize libraries that work with them.

::: browser/devtools/memory/test/unit/head.js
@@ +41,5 @@
> +  ThreadSafeChromeUtils.saveHeapSnapshot(path, { debugger: this.dbg });
> +  return HeapSnapshotFileUtils.getSnapshotIdFromPath(path);
> +}), "saveHeapSnapshot");
> +
> +function waitForState (store, predicate) {

This util function makes a lot of sense, but we'll probably more often want to wait on a specific action. I have a util function for that: https://github.com/jlongster/gecko-dev/blob/debugger-refactor-sources2/browser/devtools/debugger/test/mochitest/head.js#L1195

That uses another middleware though, which allows async action creators to actually block until another action happens (should be used rarely but very useful when it's needed).

All this middleware should work perfectly with Redux too, it uses the same API (if there's a difference we should fix it)

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201949-memory-fluxify.patch

Restructured the async action creator for taking a snapshot -- how's this structure?

Comment 5

[James Long (:jlongster)]

Comment on attachment 8664936 [details] [diff] [review]
1201949-memory-fluxify.patch

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

Looks great to me, don't worry too much about the mutability comment, we just need to talk about it more. It will come up when we start using React more, but it's ok for now, just something to think about.

My only major concern is the waitUntil polling function, polling like that is really bad and I don't see where you use it. Since you don't use it, can you remove it?

::: browser/devtools/memory/actions/snapshot.js
@@ +9,5 @@
> +let SNAPSHOT_ID_INC = 0;
> +const takeSnapshot = exports.takeSnapshot = function takeSnapshot (front) {
> +  return {
> +    type: actions.TAKE_SNAPSHOT,
> +    id: SNAPSHOT_ID_INC++,

You shouldn't need this anymore, right? The promise middleware adds a `seqId` field to each unique request. Unless you want to track IDs numerically yourself, you can just use `action.seqId`.

::: browser/devtools/memory/reducers/snapshot.js
@@ +16,5 @@
> +        console.error(`No snapshot with id "${id}" for TAKE_SNAPSHOT`);
> +        break;
> +      }
> +      snapshot.status = "done";
> +      snapshot.snapshotId = action.value;

Especially now that we are using Redux, you don't want to mutate like this. When we start using React, you won't even see updates on the screen if you don't always return a new object (because it will think nothing has changed, by just doing an === equality check which is really fast).

This is probably fine for now though if you don't want to think about it much more, and we can talk about immutability soon. I'm actually still mutating in my debugger refactor because it was easier to port code that way, but was planning on cleaning it up.

@@ +17,5 @@
> +        break;
> +      }
> +      snapshot.status = "done";
> +      snapshot.snapshotId = action.value;
> +      return Object.assign(state);

`Object.assign` does not, sadly, create a new object. It mutates the first parameter.

var a = {};
Object.assign(a) === a; // true

I think we should make a `merge` util function that never mutates the first param, I've needed it tons of times but ended up doing `Object.assign({}, foo, { ... })`. It's really unfortunate that `Object.assign` mutates.

@@ +22,5 @@
> +
> +    case "error":
> +      console.error(action);
> +  }
> +  return Object.assign(state);

ditto

::: toolkit/devtools/DevToolsUtils.js
@@ +184,5 @@
> + *        Invoked once in a while until it returns true.
> + * @param number interval [optional]
> + *        How often the predicate is invoked, in milliseconds.
> + */
> +exports.waitUntil = function waitUntil (predicate, interval = 10) {

What is this? I really don't like the polling behavior here. I didn't see where it was used.

If you're trying to replace the WAIT_UNTIL service with this, I much prefer the service because there is a specific order to how it executes things (i.e. keeping everything synchronous). And polling is really bad.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8664936 [details] [diff] [review]
1201949-memory-fluxify.patch

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

::: browser/devtools/memory/actions/snapshot.js
@@ +9,5 @@
> +let SNAPSHOT_ID_INC = 0;
> +const takeSnapshot = exports.takeSnapshot = function takeSnapshot (front) {
> +  return {
> +    type: actions.TAKE_SNAPSHOT,
> +    id: SNAPSHOT_ID_INC++,

I guess the seqId should be unique, sounds good

::: browser/devtools/memory/reducers/snapshot.js
@@ +17,5 @@
> +        break;
> +      }
> +      snapshot.status = "done";
> +      snapshot.snapshotId = action.value;
> +      return Object.assign(state);

We can use require('sdk/object/util').merge({}, ...) for now, but another utility works too so we don't need to specify a {} every time

::: browser/devtools/memory/test/unit/test_action-take-snapshot.js
@@ +38,5 @@
> +    }
> +  }
> +
> +  controller.dispatch(actions.takeSnapshot(front));
> +  yield waitUntil(() => foundPendingState && foundDoneState);

Here's where `waitUntil` is used -- it's also used in a few other tools' head files (just replaced performance/test/head.js for now) -- used only in tests

::: toolkit/devtools/DevToolsUtils.js
@@ +184,5 @@
> + *        Invoked once in a while until it returns true.
> + * @param number interval [optional]
> + *        How often the predicate is invoked, in milliseconds.
> + */
> +exports.waitUntil = function waitUntil (predicate, interval = 10) {

Commented in the test file where it's used -- this is used in a few test suites, it's useful in tests because it lets us black box test, easier to refactor (don't need to know what internal events are fired, just observe some expected state) and again, used only in tests

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

If it is only for tests, perhaps you could put it in a shared head.js file and reference it from the test's ini.

Comment 8

[James Long (:jlongster)]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #6)
> ::: toolkit/devtools/DevToolsUtils.js
> @@ +184,5 @@
> > + *        Invoked once in a while until it returns true.
> > + * @param number interval [optional]
> > + *        How often the predicate is invoked, in milliseconds.
> > + */
> > +exports.waitUntil = function waitUntil (predicate, interval = 10) {
> 
> Commented in the test file where it's used -- this is used in a few test
> suites, it's useful in tests because it lets us black box test, easier to
> refactor (don't need to know what internal events are fired, just observe
> some expected state) and again, used only in tests

Seems like you could instead just do `store.subscribe` and add a listener for when state changes, and run the predicate every time there, and when it's true resolve the promise and unsubscribe the listener.

var unsubscribe = store.subscribe(() => {
  if(predicate(store.getState())) {
    unsubscribe();
    resolve()
  }
})

Code written in a bugzilla comment so not guaranteed to work

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8664936 [details] [diff] [review]
1201949-memory-fluxify.patch

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

I wanted to see how this brave new redux world works, so I looked through the code. Pretty cool stuff :) I have a couple questions, and a couple nits.

Sorry for the drive by!

::: browser/devtools/memory/initializer.js
@@ +18,5 @@
> + */
> +let controller = null;
> +function initialize () {
> +  return Task.spawn(function *() {
> +    controller = new MemoryController({ toolbox: gToolbox, target: gTarget, front: gFront });

Are you purposefully making this a global? If so, add a "var controller;" at the top level (let/const do not create global properties).

::: browser/devtools/memory/reducers/snapshot.js
@@ +10,5 @@
> +        status: action.status
> +      }];
> +
> +    case "done":
> +      let snapshot = state.find(s => s.id === action.id);

Why keep the snapshots in an array if you are querying by id? An object/map from id -> snapshot will provide better querying and equivalent iteration and "immutable insertion" (mutable insertion would be faster as well, but... we should probably just import immutable.js).

@@ +12,5 @@
> +
> +    case "done":
> +      let snapshot = state.find(s => s.id === action.id);
> +      if (!snapshot) {
> +        console.error(`No snapshot with id "${id}" for TAKE_SNAPSHOT`);

Nit: DevToolsUtils.reportException

@@ +17,5 @@
> +        break;
> +      }
> +      snapshot.status = "done";
> +      snapshot.snapshotId = action.value;
> +      return Object.assign(state);

Why do you even need to Object.assign(state) when you have already done the mutations on state? Was this meant to copy, like the comments above discuss, or am I missing something?

FWIW, its easy to define a helper `immutableAssign(...objs) => Object.assign({}, ...objs)` so the "ugliness" of specifying `{}` everytime is moot, but yeah we really want something like immutable.js.

@@ +20,5 @@
> +      snapshot.snapshotId = action.value;
> +      return Object.assign(state);
> +
> +    case "error":
> +      console.error(action);

Nit: DevToolsUtils.reportException

::: browser/devtools/memory/test/unit/head.js
@@ +41,5 @@
> +  this.state = "detached";
> +});
> +
> +StubbedMemoryFront.prototype.saveHeapSnapshot = expectState("attached", Task.async(function *() {
> +  const path = ThreadSafeChromeUtils.saveHeapSnapshot({ debugger: this.dbg });

If the whole stubbed memory front is just to get a debugger for use right here, you could just do { runtime: true } instead and you wouldn't need any debugger or stubbed memory front or anything.

::: browser/devtools/shared/redux/middleware/promise.js
@@ +6,5 @@
> +const uuidgen = require("sdk/util/uuid").uuid;
> +const {
> +  entries, toObject, reportException, executeSoon
> +} = require("devtools/toolkit/DevToolsUtils");
> +const PROMISE = exports.PROMISE = "@@dispatch/promise";

Why not use Symbol("dispatch/promise") ?

@@ +15,5 @@
> +      return next(action);
> +    }
> +
> +    const promise = action[PROMISE];
> +    const seqId = uuidgen().toString();

Do we need full uuids here, or would a monotonically increasing integer be good enough?

@@ +33,5 @@
> +          value: value
> +        }));
> +      });
> +    }).catch(error => {
> +      executeSoon(() => {

Why executeSoon(...) here and above? Unless you are using deprecated-sync-thenables (which we shouldn't be adding more uses of) then .then/.catch/etc are always executed on a different tick of the event loop already. If I'm missing something, then we definitely need an explanatory comment!

::: toolkit/devtools/DevToolsUtils.js
@@ +139,5 @@
> + */
> +exports.toObject = function(arr) {
> +  const obj = {};
> +  for(let pair of arr) {
> +    obj[pair[0]] = pair[1];

Nit: space after "for" before "(".

If you prefer:

  for (let [k, v] of arr) {
    obj[k] = v;
  }

@@ +184,5 @@
> + *        Invoked once in a while until it returns true.
> + * @param number interval [optional]
> + *        How often the predicate is invoked, in milliseconds.
> + */
> +exports.waitUntil = function waitUntil (predicate, interval = 10) {

At minimum, this should have a big comment like:

  // XXX: DO NOT USE THIS FOR ANYTHING EXCEPT TESTS

But I think putting it in a shared head.js file is the way to go.

Comment 10

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #9)
> Comment on attachment 8664936 [details] [diff] [review]
> 1201949-memory-fluxify.patch
> 
> Review of attachment 8664936 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I wanted to see how this brave new redux world works, so I looked through
> the code. Pretty cool stuff :) I have a couple questions, and a couple nits.
> 
> Sorry for the drive by!
> 
> ::: browser/devtools/memory/initializer.js
> @@ +18,5 @@
> > + */
> > +let controller = null;
> > +function initialize () {
> > +  return Task.spawn(function *() {
> > +    controller = new MemoryController({ toolbox: gToolbox, target: gTarget, front: gFront });
> 
> Are you purposefully making this a global? If so, add a "var controller;" at
> the top level (let/const do not create global properties).

Aaaaand I just saw the `let controller;`. Should make that a var if you want it on the global scope. If you have questions, ask shu and he can tell you about it in between sobs.

Comment 11

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8664936 [details] [diff] [review]
1201949-memory-fluxify.patch

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

::: browser/devtools/memory/initializer.js
@@ +18,5 @@
> + */
> +let controller = null;
> +function initialize () {
> +  return Task.spawn(function *() {
> +    controller = new MemoryController({ toolbox: gToolbox, target: gTarget, front: gFront });

+1

::: browser/devtools/memory/reducers/snapshot.js
@@ +10,5 @@
> +        status: action.status
> +      }];
> +
> +    case "done":
> +      let snapshot = state.find(s => s.id === action.id);

We can't store Maps in the Redux state (AFAIK) -- I think James is adding some immutable.js stuff, but not using that here yet. An object could work here for IDs but we'd lose ordering (even though key ordering is codified for Objects). Look up will never be an issue here, and I think arrays are more semantic in this case

::: browser/devtools/shared/redux/middleware/promise.js
@@ +6,5 @@
> +const uuidgen = require("sdk/util/uuid").uuid;
> +const {
> +  entries, toObject, reportException, executeSoon
> +} = require("devtools/toolkit/DevToolsUtils");
> +const PROMISE = exports.PROMISE = "@@dispatch/promise";

We can't use Symbols for actions, but I think as this is a property it could be a symbol -- James?

@@ +15,5 @@
> +      return next(action);
> +    }
> +
> +    const promise = action[PROMISE];
> +    const seqId = uuidgen().toString();

I'm guessing just monotonically increasing ints would be sufficient, but this was made by James

@@ +33,5 @@
> +          value: value
> +        }));
> +      });
> +    }).catch(error => {
> +      executeSoon(() => {

My understanding via James was that this takes it out of the promise-cycle, so throwing here isn't handled within the promise but within the handling here -- not sure which way is better/makes more sense, but yes, definitely a comment here regardless

::: toolkit/devtools/DevToolsUtils.js
@@ +139,5 @@
> + */
> +exports.toObject = function(arr) {
> +  const obj = {};
> +  for(let pair of arr) {
> +    obj[pair[0]] = pair[1];

This was migrated over from one of james' patches -- I'll update this

Comment 12

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8664936 [details] [diff] [review]
1201949-memory-fluxify.patch

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

::: browser/devtools/memory/reducers/snapshot.js
@@ +10,5 @@
> +        status: action.status
> +      }];
> +
> +    case "done":
> +      let snapshot = state.find(s => s.id === action.id);

Why can't we store maps in the state?

Why would we lose ordering even though iteration order of keys in objects is well-defined?

::: browser/devtools/shared/redux/middleware/promise.js
@@ +6,5 @@
> +const uuidgen = require("sdk/util/uuid").uuid;
> +const {
> +  entries, toObject, reportException, executeSoon
> +} = require("devtools/toolkit/DevToolsUtils");
> +const PROMISE = exports.PROMISE = "@@dispatch/promise";

Why can't we use Symbols for actions? Because they get passed to a switch statement and stringified?

@@ +33,5 @@
> +          value: value
> +        }));
> +      });
> +    }).catch(error => {
> +      executeSoon(() => {

Where does the error handling happen with the executeSoon(...) code now? Should these be wrapped in DevToolsUtils.makeInfallible?

Comment 13

[James Long (:jlongster)]

(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #12)
> Comment on attachment 8664936 [details] [diff] [review]
> 1201949-memory-fluxify.patch
> 
> Review of attachment 8664936 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/memory/reducers/snapshot.js
> @@ +10,5 @@
> > +        status: action.status
> > +      }];
> > +
> > +    case "done":
> > +      let snapshot = state.find(s => s.id === action.id);
> 
> Why can't we store maps in the state?
> 
> Why would we lose ordering even though iteration order of keys in objects is
> well-defined?
> 
> ::: browser/devtools/shared/redux/middleware/promise.js
> @@ +6,5 @@
> > +const uuidgen = require("sdk/util/uuid").uuid;
> > +const {
> > +  entries, toObject, reportException, executeSoon
> > +} = require("devtools/toolkit/DevToolsUtils");
> > +const PROMISE = exports.PROMISE = "@@dispatch/promise";
> 
> Why can't we use Symbols for actions? Because they get passed to a switch
> statement and stringified?

Action types can't be symbols, but this is just a property on the action so it could work. My only concern is serializing them; now we need to make sure we serialize it to a string and then unserialize it to a symbol when we serialize/deserialize actions. However, eventually if we use immutable-js we may need a smart serializer anyway. I'm fine changing this to a symbol right now if we want and than figure out stuff later.

> 
> @@ +33,5 @@
> > +          value: value
> > +        }));
> > +      });
> > +    }).catch(error => {
> > +      executeSoon(() => {
> 
> Where does the error handling happen with the executeSoon(...) code now?
> Should these be wrapped in DevToolsUtils.makeInfallible?

Basically it's a question of what happens in sync functions that are outside of promises. If we always want top-level error handling, we can't depend on promises for that anyway, because even in the old debugger there is a bunch of code that is run outside of promises.

There's a *big* development win here in that most code is now outside of promises, and we see all errors normally and can use "break on exception" properly, etc.

We can install a global error handler though in production though by installing it in a middleware. Basically whenever an action is dispatched it would run in a try/catch and there we can do something with it. But we'll turn this off while in development, and maybe in tests.

function errorHandlerMiddleware(dispatch) {
  return next => action => {
    try {
      dispatch(action);
    }
    catch(e) {
      // ...
    }
  };
}

All changes must go through a dispatch, and the UI is synchronously updated in that `dispatch` call so it should cover everything

Comment 14

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201949-memory-fluxify.patch

Addressed all the comments -- and some shelved for future discussions (as it seems a bit unclear how all these things are going to tie together with redux/devtools/react at this time). Thanks James & Nick!

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

Comment 15

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201949-memory-fluxify.patch

Some dangling migration failures, try #2:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a26d83da7d78

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/f03e855428e7

Comment 17

[Carsten Book [:Tomcat]]

backed out for test failures like :

https://treeherder.mozilla.org/logviewer.html#?job_id=4828543&repo=fx-team

Comment 18

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/fx-team/rev/ad18f16fe74a

Comment 19

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1201949-memory-fluxify.patch

More migration fixes, try running on all tests now:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=642acc9d920e

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c035b83f7c4b

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c035b83f7c4b