Closed Bug 1240981 Opened 4 years ago Closed 4 years ago

Use BrowserLoader in performance tools

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect)

defect
Not set

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: jsantell, Assigned: jsantell)

References

Details

Attachments

(1 file, 1 obsolete file)

Necessary for react components, for JIT view.
This is for using React/Redux.

* Removed all lazy loading; browser loader doesn't have that, and even if we did add that, it wouldn't be compatible with browserify/webpack-ish future. Most of these were already loaded anyway on start.
* marker-formatters was pulled out to avoid circular dependency after lazy loading was gone
* Manually unbind the preference listener -- for some reason, browser loader exposes this error; this was the cause of those leaks.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66dd543781fb
Attachment #8710247 - Flags: review?(nfitzgerald)
Comment on attachment 8710247 [details] [diff] [review]
1240981-perf-browser-loader.patch

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

::: devtools/client/performance/performance-controller.js
@@ +3,5 @@
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  "use strict";
>  
>  var { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components;
> +//var { loader, require } = Cu.import("resource://devtools/shared/Loader.jsm", {});

already removed
Some xpcshell tests failing because some components used the special observable PREFS object, and when the controller registers/unregisters now, the standalone unit tests never call the controller, meaning these getters are never loaded. So just using Services.prefs in these cases in marker-utils/marker-formatter.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f8633f954b8
Attachment #8710247 - Attachment is obsolete: true
Attachment #8710247 - Flags: review?(nfitzgerald)
Attachment #8710292 - Flags: review?(nfitzgerald)
Comment on attachment 8710292 [details] [diff] [review]
1240981-perf-browser-loader.patch

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

Nice digging to find that leak!
Attachment #8710292 - Flags: review?(nfitzgerald) → review+
https://hg.mozilla.org/mozilla-central/rev/97866a808db2
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.