In bug 1219073, we pass the toolbox into the heap tree for rendering, but we should pass in a onClick callback, wrapping toolbox in a closure. A small nice refactoring.

I want to use the frame component in jit view, so taking care of this

Happy holidays! Here's a huge, confusing patch. * Frame view component pulled out of memory tool (and onClick function and L10N things are passed in now) * Previous "shared/source-utils.js" which handles opening a source in different tools now moved to "shared/view-source.js" to be more descriptive * "shared/source-utils.js" now contains a handful of source utilities from performance's "performance/modules/logic/frame-utils.js". This paves the way for using this component in jit view, perf call tree and console, etc., which gives us a common component to handle source mapping as well. https://treeherder.mozilla.org/#/jobs?repo=try&revision=bec32e4d2e9d

Also normalized our usage of host vs hostname, etc., was used inconsistently. Something to look over/think about is our terminology of location, frame, and source, and different representations of each (like call tree for example doesn't used SavedFrames, but generates it out of a string). I think it's consistent for the most part now, but "parseLocation" in frame-utils maybe should be something different, like stackToFrame or something.

Comment on attachment 8701592 [details] [diff] [review] 1219544-decouple-frame-view.patch Review of attachment 8701592 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for cleaning this up! r=me with the change of name to fit in line with all of our other event handlers. ::: devtools/client/memory/app.js @@ +97,5 @@ > HeapView({ > snapshot: selectedSnapshot, > diffing, > onSnapshotClick: () => dispatch(takeSnapshotAndCensus(front, heapWorker)), > + linkToDebugger: (frame) => toolbox.viewSourceInDebugger(frame.source, frame.line), Nit: no need for () around single param. ::: devtools/client/memory/components/heap.js @@ +90,5 @@ > > propTypes: { > onSnapshotClick: PropTypes.func.isRequired, > snapshot: snapshotModel, > + linkToDebugger: PropTypes.func.isRequired, All of our event handlers are named "on<action>", and so for consistency this should be named something like "onViewSourceInDebugger".

[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