Closed Bug 1060464 Opened 6 years ago Closed 6 years ago

--jsdebugger switch in mochitests crashes immediately because of non-local connections (debugging profile gets the env var telling us to crash, don't get alternative prefs so we don't hit the network so much)

Categories

(DevTools :: Framework, defect)

defect
Not set
critical

Tracking

(firefox34 fixed, firefox35 fixed)

RESOLVED FIXED
Firefox 35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: tomasz, Assigned: Gijs)

References

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

I tried

mach mochitest-chrome tests/chrome/test_findbar.xul --jsdebugger

but the browser toolbox (--jsdebugger) window immediately crashes with

3 INFO FATAL ERROR: Non-local network connections are disabled and a connection attempt to tiles.services.mozilla.com (54.186.213.109) was made.
4 INFO You should only access hostnames available via the test networking proxy (if running mochitests) or from a test-specific httpd.js server (if running xpcshell tests). Browser services should be disabled or redirected to a local server.

I talked to :Gijs and it looks like it's a regression introduced in bug 1042876.
Ed, can we add dummy prefs for the tiles thing you added in bug 1042876 so that this doesn't happen? (well, assuming that that fixes this)

I'm not sure why only reftests were fixed.
Blocks: 1042876
Flags: needinfo?(edilee)
Severity: normal → critical
Keywords: regression
Component: Developer Tools: Debugger → New Tab Page
OS: Mac OS X → All
Hardware: x86 → All
I can confirm that dropping

https://hg.mozilla.org/integration/fx-team/file/8f49394f8069/browser/app/profile/firefox.js#l1539
and
https://hg.mozilla.org/integration/fx-team/file/8f49394f8069/browser/app/profile/firefox.js#l1542

"fixes" my issue.
No longer blocks: 1042876
Severity: critical → normal
Component: New Tab Page → Developer Tools: Debugger
Keywords: regression
OS: All → Mac OS X
Hardware: All → x86
Blocks: 1042876
Severity: normal → critical
Component: Developer Tools: Debugger → New Tab Page
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All
Where do these preferences need to be set?

They've been set in these files:
- addon-sdk/source/python-lib/cuddlefish/prefs.py
- layout/tools/reftest/runreftest.py
- testing/profiles/prefs_general.js
- testing/talos/talos.json -> talos/PerfConfigurator.py
(In reply to Ed Lee :Mardak from comment #3)
> Where do these preferences need to be set?
> 
> They've been set in these files:
> - addon-sdk/source/python-lib/cuddlefish/prefs.py
> - layout/tools/reftest/runreftest.py
> - testing/profiles/prefs_general.js
> - testing/talos/talos.json -> talos/PerfConfigurator.py

Oh! I'm sorry, these weren't in the patch on the bug (v2)... that then sounds like these aren't being used for the debugging profile. We should probably either fix the debugger code to use those prefs for its new profile (sounds hard) or fix the mochitest "die remote access die" code to not run against the debugger process... which actually also sounds hard, because it looks like it's triggered by an env var ("MOZ_DISABLE_NONLOCAL_CONNECTIONS"), which will be propagated to the debugging process started by the mainline firefox process...
Flags: needinfo?(edilee)
Dave, could we copy the prefs.js file from the main profile to the test profile when creating a debugging profile? That'd basically fix this issue immediately, and potentially have some benefits in terms of the prefs you'd have in your main profile regarding debugging and devtools and so on would persist in the new chrome debugging profile...
Component: New Tab Page → Developer Tools: Framework
Flags: needinfo?(dcamp)
Summary: mochitest-chrome does not work with jsdebugger → --jsdebugger switch in mochitests crashes immediately because of non-local connections (debugging profile gets the env var telling us to crash, don't get alternative prefs so we don't hit the network so much)
No longer blocks: 1042876
Yeah that seems reasonable.
Flags: needinfo?(dcamp)
The prefs file sync writing hack is icky, but it works and the difference with nsIFile.copyTo here is probably slim-to-none, nevermind the difference with calling createProfile, which is what we did before. I don't feel comfortable also making all of this async in this changeset, so we'll leave that for another time, if that's OK...
Attachment #8481682 - Flags: review?(dcamp)
Attachment #8481682 - Flags: review?(benjamin)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Fixed up octal syntax, got rid of one clone() - left the other for clarity...
Attachment #8481709 - Flags: review?(dcamp)
Attachment #8481709 - Flags: review?(benjamin)
Attachment #8481682 - Attachment is obsolete: true
Attachment #8481682 - Flags: review?(dcamp)
Attachment #8481682 - Flags: review?(benjamin)
Attachment #8481709 - Flags: review?(benjamin) → review+
Comment on attachment 8481709 [details] [diff] [review]
fix the --jsdebugger flag when running tests, also fixes bug 1060493,

23:16	<dcamp>	Gijs: away from my computer atm, but you have my r+ if you have bsmedberg's
23:17	<dcamp>	Gijs: that patch was outside of my comfort zone and I should have passed it off, sorry
23:17	<dcamp>	Gijs: but if it works, it can't be worse than what I'd there
Attachment #8481709 - Flags: review?(dcamp) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/c1319f301eb0
Whiteboard: [fixed-in-fx-team]
Sigh - I shouldn't have assumed that the private properties were only in use in the jsm, clearly... I've adapted the test, and sent this to try: https://tbpl.mozilla.org/?tree=Try&rev=8697718dc0a0
Comment on attachment 8483738 [details] [diff] [review]
fix the --jsdebugger flag when running tests, also fixes bug 1060493,

Carrying over review...
Attachment #8483738 - Flags: review+
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8481709 - Attachment is obsolete: true
Duplicate of this bug: 1062553
Did this reland in fx-team or not? I'm still getting the FATAL ERROR message on the latest fx-team. Did we just forget to remove [fixed-in-fx-team]?
Checked this in as https://hg.mozilla.org/integration/fx-team/rev/303c2817e129

If this fails again, Gijs, can you make sure the try run also tests against mochitest-dt in addition to mochitest-bc?
(In reply to Wes Kocher (:KWierso) from comment #16)
> Checked this in as
> https://hg.mozilla.org/integration/fx-team/rev/303c2817e129
> 
> If this fails again, Gijs, can you make sure the try run also tests against
> mochitest-dt in addition to mochitest-bc?

*bangs head against wall*

Sigh. Good thing the local testing seems to have been representative, as fx-team looks green. Thanks for checking this in! :-)
https://hg.mozilla.org/mozilla-central/rev/303c2817e129
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Toolbox crashes after this change with:

System JS : ERROR resource:///modules/devtools/ToolboxProcess.jsm:162 - ReferenceError: Cr is not defined

It looks like you've forgotten:

const Cr = Components.results;

Repro:
0. make clean profile
1. mach run --jsdebugger
2. close everything
3. mach run --jsdebugger
4. crash
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(This shows that we really need a test for this :) )
This fixes the issue for me.
Attachment #8484487 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8484487 [details] [diff] [review]
forgotten-cr.patch

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

rs=me
Attachment #8484487 - Flags: review?(gijskruitbosch+bugs) → review+
I'm not sure what rs=me means. BTW, would you submit it or should I checkin-needed' it?
remote:   https://hg.mozilla.org/integration/fx-team/rev/ad6298e44579


(In reply to Tomasz Kołodziejski [:tomasz] from comment #23)
> I'm not sure what rs=me means. BTW, would you submit it or should I
> checkin-needed' it?

rs=rubber-stamp

I just landed this (see above).

(In reply to Girish Sharma [:Optimizer] from comment #20)
> (This shows that we really need a test for this :) )

We have a test; the issue Tomasz found only shows up the second time you launch the browser debugger for the same profile. Feel free to file a followup bug to test that.
https://hg.mozilla.org/mozilla-central/rev/ad6298e44579
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1063927
Nick, is this worth uplifting to aurora (considering this broke slightly before 34 branched, and 34's aurora branch is young yet), or shall we just let it ride the trains?
Flags: needinfo?(nfitzgerald)
Yes, let's uplift.
Flags: needinfo?(nfitzgerald)
Attachment #8483738 - Attachment is obsolete: true
Comment on attachment 8485807 [details] [diff] [review]
Combined patch for Aurora

Approval Request Comment
[Feature/regressing bug #]: long-standing bug became acute when we landed prefs which caused network connections on startup on new profiles (bug 1042876)
[User impact if declined]: can't really use jsdebugger to debug mochitests
[Describe test coverage new/current, TBPL]: some automated test coverage, but could be better...
[Risks and why]: fundamental aspect of how we run the jsdebugger changed, which is inherently risky - but equally, this gets used by frontend and devtools folks pretty heavily, and we would have noticed by now if it wasn't properly fixed (but is definitely broken on 34 right now), and 34 is still very young
[String/UUID change made/needed]: none
Attachment #8485807 - Flags: review+
Attachment #8485807 - Flags: approval-mozilla-aurora?
Comment on attachment 8485807 [details] [diff] [review]
Combined patch for Aurora

It is still early in the Aurora 34 cycle. Taking this to facilitate dev work. Aurora+
Attachment #8485807 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.