Open Bug 979101 Opened 6 years ago Updated 2 years ago

Enable remote debugging by default for local builds

Categories

(Firefox for Android :: General, defect, P5)

ARM
Android
defect

Tracking

()

REOPENED
Firefox 30

People

(Reporter: Margaret, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I blow away my profile often for local builds, and it's annoying to flip this pref.
Actually, why don't we even enable this by default on all channels? A user needs to have USB debugging turned on on their phone for this to work, and that's definitely way more of a security risk.  Also, there's even a dialog that appears asking you to approve the connection.

I suppose when you flip the pref we tell you about setting up port forwarding, but you can figure that out if you're trying to get remote debugging working for the first time. Also, if you have multiple instances of Fennec running on your device, all with remote debugging enabled, the wrong one can be connected.

I think we should make an effort to make it as easy as possible for web developers to test/develop in Firefox for Android.
Since this is a developer feature, I'm not sure what kind of product approval we need here, but I think this change will make life nicer for developers, and since it only affects Nightly, it feels like it isn't a big deal to change.
Attachment #8389402 - Flags: review?(mark.finkle)
Comment on attachment 8389402 [details] [diff] [review]
Enable remote debugger by default on nightly builds

Since this only affects Nightly, I agree.
Attachment #8389402 - Flags: review?(mark.finkle) → review+
Much happiness. Such adb forward. Wow.
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/1acaa42690a6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Backed out for memory regressions :(

https://hg.mozilla.org/integration/fx-team/rev/6a184aee9ea0

Let's just WONTFIX this. Developers like me are probably few, and we can figure out some local hack to streamline this process (like just changing this in our own mobile.js when building).

I had assumed this was disabled for security reasons, but obviously memory reasons are more of a factor here.
Resolution: FIXED → WONTFIX
FYI, it also looks like backing this out fixed a talos regression. So another reason not to do this.
(In reply to :Margaret Leibovic from comment #7)
> Backed out for memory regressions :(
> 
> https://hg.mozilla.org/integration/fx-team/rev/6a184aee9ea0
> 
> Let's just WONTFIX this. Developers like me are probably few, and we can
> figure out some local hack to streamline this process (like just changing
> this in our own mobile.js when building).
> 
> I had assumed this was disabled for security reasons, but obviously memory
> reasons are more of a factor here.

Given this context, perhaps we should enable this by default just for local builds (using MOZILLA_OFFICIAL as our "in-automation" flag) and maybe for Nightly debug builds.  

See, for example, http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AndroidManifest.xml.in#88.
As discussed on IRC today, let's make another attempt at this.  We'll restrict it to local builds only as :nalexander suggests in comment 9.

He also suggested we should toast on connection (instead of prompting) for the local build case.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Assignee: margaret.leibovic → nobody
Kinda looks like we might want, from b2g/simulator/custom-prefs.js:

1 	user_pref("devtools.debugger.prompt-connection", false);
2 	user_pref("devtools.debugger.forbid-certified-apps", false);
3 	user_pref("devtools.apps.forbidden-permissions", "");
7 	user_pref("devtools.debugger.remote-enabled", true);
8 	user_pref("devtools.chrome.enabled", true);

So, around https://dxr.mozilla.org/mozilla-central/source/mobile/android/app/mobile.js#435, we'll want a conditional block to flip some subset of those things above.
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.