Closed
Bug 771056
Opened 12 years ago
Closed 12 years ago
Profile dump (temporary) path should not be hardcoded
Categories
(Core :: Gecko Profiler, defect)
Core
Gecko Profiler
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: glandium, Assigned: glandium)
Details
Attachments
(2 files, 1 obsolete file)
2.75 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
3.34 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
Instead, the directory service should be used (probably requesting for NS_OS_TEMP_DIR).
Assignee | ||
Comment 1•12 years ago
|
||
This requires a change to the addon that I'll attach here too.
Attachment #640259 -
Flags: review?(bgirard)
Assignee | ||
Comment 2•12 years ago
|
||
Assignee: nobody → mh+mozilla
Attachment #640260 -
Flags: review?(bgirard)
Comment 3•12 years ago
|
||
Comment on attachment 640260 [details] [diff] [review] Get profile dumps from NS_OS_TEMP_DIR location on android and b2g Review of attachment 640260 [details] [diff] [review]: ----------------------------------------------------------------- ::: lib/main.js @@ +522,5 @@ > // now pull the fennec libs themselves > cmdRunnerModule.runCommand("/bin/bash -l -c 'rm tmp/symbol.apk'", function (r) {}); > cmdRunnerModule.runCommand( > + "/bin/bash -l -c '" + > + ["adb pull " + path + "/profile_0_" + processInfo.pid + ".txt /tmp/fennec_profile.txt" for each (path in processInfo.savePath)].join('||') + ";" + I really don't like the use for [] for each join. Can we write this code to be simpler?
Comment 4•12 years ago
|
||
Comment on attachment 640259 [details] [diff] [review] Use NS_OS_TEMP_DIR to save SPS profile data I was going to ask what directory they would end up in but the next patch answers my question.
Attachment #640259 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #640260 -
Attachment is obsolete: true
Attachment #640260 -
Flags: review?(bgirard)
Attachment #640300 -
Flags: review?(bgirard)
Comment 6•12 years ago
|
||
Why are you no longer checking /sdcard? Couldn't you just moved the code to a helper function where it could of been written cleanly?
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #6) > Why are you no longer checking /sdcard? /sdcard is still checked.
Comment 8•12 years ago
|
||
Comment on attachment 640300 [details] [diff] [review] Get profile dumps from NS_OS_TEMP_DIR location on android and b2g Right. I see it now. I'll land this change when you land the first on m-c.
Attachment #640300 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 9•12 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #8) > Comment on attachment 640300 [details] [diff] [review] > Get profile dumps from NS_OS_TEMP_DIR location on android and b2g > > Right. I see it now. I'll land this change when you land the first on m-c. It would make more sense to do it the other way around. The addon change explicitely keeps backwards compatibility with the current m-c behaviour.
Comment 10•12 years ago
|
||
Release 1.7.17 https://github.com/bgirard/Gecko-Profiler-Addon/commit/29e2062ea81a9224bd102d589fb52dd8f7ae2cab
Assignee | ||
Comment 11•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/cdd36f5c088b
Target Milestone: --- → mozilla16
Comment 12•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cdd36f5c088b
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•