Closed
Bug 1016089
Opened 10 years ago
Closed 7 years ago
Allow TART to control the profiler outside of talos
Categories
(Testing :: Talos, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: avih, Assigned: avih)
References
Details
(Keywords: ateam-talos-task)
Attachments
(1 file, 1 obsolete file)
4.73 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #967635 +++ The first fix (bug 1010473) only prevented complete breakage outside talos, but without making sure that the profiler is actually being controlled during the run. Profiler.js (pause/resume the profiler in talos and TART) contains the following code as part of its setup: > netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect'); This is no longer required nor allowed for addons since Firefox 17: https://developer.mozilla.org/en-US/Firefox/Releases/17/Site_compatibility#Security Therefore, when running TART outside of talos, the setup fails and the rest of the code at Profiler.js becomes no-op (TART still runs and measures, but can't control the profiler nor add markers). However, we apparently still need this code at Profiler.js to control profiler pause/resume from non privileged pages which talos loads and tests, like the tscroll pages. In order to make it work, the talos run logs show (what probably allows the privileges request to work): > security.turn_off_all_security_so_that_viruses_can_take_over_this_computer: true Off topic: The preference itself is either deprecated or replaced with security.enablePrivilege.enable_for_tests (also see at the Firefox 17 security notes from the above link). My solution is to put the privileges request inside a try/catch block, such that it will work where needed (and can work - like inside talos) and wouldn't break the setup where it's not allowed to be invoked - such as during TART runs outside of talos.
Assignee | ||
Comment 1•10 years ago
|
||
This also reverts part of the patch of bug 1010473. I think it was redundant already when it landed.
Attachment #8429481 -
Flags: review?(jmaher)
Comment 2•10 years ago
|
||
Comment on attachment 8429481 [details] [diff] [review] v1 Puts the privileges request inside try/catch Review of attachment 8429481 [details] [diff] [review]: ----------------------------------------------------------------- if we change this, I want to change the other 2 files as well! ::: talos/page_load_test/tart/addon/content/Profiler.js @@ +27,5 @@ > + // It's used inside talos from non-privileged pages (like during tscroll), > + // and it works because talos disables all/most security measures. > + netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect'); > + } catch (e) { > + } we we do this outside of the first try, so we don't have nested try blocks?
Attachment #8429481 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 3•10 years ago
|
||
Apologies, I accidentally misread and thought it's an r+ with the changes you requested. I modified as requested (new attachment) and pushed: https://hg.mozilla.org/build/talos/rev/2b78501af8d0 Sorry. But do please review this as well to make sure nothing slipped. Thanks.
Attachment #8429481 -
Attachment is obsolete: true
Attachment #8429584 -
Flags: review?(jmaher)
Comment 4•10 years ago
|
||
Comment on attachment 8429584 [details] [diff] [review] v2 address review comments Review of attachment 8429584 [details] [diff] [review]: ----------------------------------------------------------------- thanks for this!
Attachment #8429584 -
Flags: review?(jmaher) → review+
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•