--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)

RESOLVED FIXED in Firefox 34

Status

defect
--
critical
RESOLVED FIXED
5 years ago
Last year

People

(Reporter: tomasz, Assigned: Gijs)

Tracking

({regression})

unspecified
Firefox 35
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox34 fixed, firefox35 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

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.
Assignee

Comment 1

5 years ago
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)
Assignee

Updated

5 years ago
Severity: normal → critical
Keywords: regression
Assignee

Updated

5 years ago
Component: Developer Tools: Debugger → New Tab Page
OS: Mac OS X → All
Hardware: x86 → All
Reporter

Comment 2

5 years ago
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
Assignee

Updated

5 years ago
Blocks: 1042876
Severity: normal → critical
Component: Developer Tools: Debugger → New Tab Page
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All

Comment 3

5 years ago
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
Assignee

Comment 4

5 years ago
(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)
Assignee

Comment 5

5 years ago
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)
Assignee

Updated

5 years ago
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)
Assignee

Updated

5 years ago
No longer blocks: 1042876
Yeah that seems reasonable.
Flags: needinfo?(dcamp)
Assignee

Comment 7

5 years ago
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

Updated

5 years ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Assignee

Comment 8

5 years ago
Fixed up octal syntax, got rid of one clone() - left the other for clarity...
Attachment #8481709 - Flags: review?(dcamp)
Attachment #8481709 - Flags: review?(benjamin)
Assignee

Updated

5 years ago
Attachment #8481682 - Attachment is obsolete: true
Attachment #8481682 - Flags: review?(dcamp)
Attachment #8481682 - Flags: review?(benjamin)

Updated

5 years ago
Attachment #8481709 - Flags: review?(benjamin) → review+
Assignee

Comment 9

5 years ago
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+
Assignee

Comment 10

5 years ago
remote:   https://hg.mozilla.org/integration/fx-team/rev/c1319f301eb0
Whiteboard: [fixed-in-fx-team]
Assignee

Comment 12

5 years ago
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
Assignee

Comment 13

5 years ago
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)
Assignee

Updated

5 years ago
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?
Assignee

Comment 17

5 years ago
(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: 5 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)
Assignee

Comment 22

5 years ago
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?
Assignee

Comment 24

5 years ago
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: 5 years ago5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1063927
Assignee

Comment 27

5 years ago
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)
Assignee

Comment 29

5 years ago
Assignee

Updated

5 years ago
Attachment #8483738 - Attachment is obsolete: true
Assignee

Comment 30

5 years ago
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+
Flags: qe-verify-
Duplicate of this bug: 986855

Updated

Last year
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.