Closed Bug 967218 Opened 7 years ago Closed 7 years ago

(Synth APK) - User visible debugging enabled notification shown

Categories

(Firefox for Android :: Web Apps (PWAs), defect)

29 Branch
ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 + fixed
firefox30 --- fixed

People

(Reporter: aaronmt, Assigned: myk)

References

Details

Attachments

(1 file)

After launching AccuWeather on my device today I noticed a 'debugging enabled' notification 962607. I am told in #mobile that APK's generated are currently marked as debug and thus is the reason. Figure we can keep this bug open until we're off the development server.
Actually we should fix this bug on the development server, as that server is user-facing, and we shouldn't be serving debug APKs to end users.

(We do want to serve debug APKs to developers, of course.  And it's confusing that we call this a "development server", because it suggests that it's the server we use to serve APKs to developers.  Whereas it's actually just the server we use to develop the server.)
dapk.net doesn't have a release and review deployment.

This will be fixed when we point Nightly at production's release environment. Does that work?
No, I don't think we should wait until we point users at production's release environment, since we want Nightly users to test the user-facing experience implemented by synthetic APKs.

(We may also want to set up a server that serves debug APKs, so webapp developers can test the developer-facing experience; and one for reviewers, as appropriate; but end-user testing is the immediate goal.)
I didn't want to have the public dev environment setup like a production deployment because
1) It's a bunch of sysadmin work for me
2) It's a continous maintenance / deployment tax

Currently dapk.net signs apks the way the prototype did, we're blocked on a stage/production deployment of the APK Signer service before we can serve APKs from a reviewer as well as a release channel.

IT is building out stage/production for APK Factory as well as APK Signer.

I suggest production, because of the following thoughts:
1) We can point Nightly at either stage or production since both will have both a reviewer and release endpoint
2) We should probably point Nightly / Aurora at production, since the system will stabilize at some point and we'd want to test changes in stage before pushing to production.

We can switch things up, this is my reasoning.
(In reply to Austin King [:ozten] from comment #4)
> I didn't want to have the public dev environment setup like a production
> deployment because
> 1) It's a bunch of sysadmin work for me
> 2) It's a continous maintenance / deployment tax

I don't want that either!


> Currently dapk.net signs apks the way the prototype did, we're blocked on a
> stage/production deployment of the APK Signer service before we can serve
> APKs from a reviewer as well as a release channel.

Sure, I understand.  What I'm suggesting is that in the meantime we serve APKs from a release channel, as our most important audience of testers right now is users (Nightly users, MWC demoers, etc.).


In any case, I dug into this further, and it turns out that dapk.net is actually generating release APKs, since its buildWithAnt function calls `ant release` to generate them:

https://github.com/mozilla/apk-factory-service/blob/e7a9b23460b342d8b932489fcc6334a96bc5a58e/lib/apk_project_builder.js#L207-L209

Thus this bug is client-side: GeckoApp.handleMessage sets mCurrentResponse to the string "false" rather than boolean false if the app is not debuggable, which eventually becomes the return value of sendMessageToJava({ type: "NativeApp:IsDebuggable" }), where it gets mistakenly treated as a boolean and thus true.

mCurrentResponse is of type String, and making it a variant is a bigger chore.  And it's going away anyway once wesj finishes standing up asynchronous messaging.  So let's fix this with a simple string comparison for the time being.
Assignee: nobody → myk
Status: NEW → ASSIGNED
Attachment #8371139 - Flags: review?(mark.finkle)
Comment on attachment 8371139 [details] [diff] [review]
patch v1: does string comparison

Good catch!
Attachment #8371139 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/e7b7825f65dd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Comment on attachment 8371139 [details] [diff] [review]
patch v1: does string comparison

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Bug 962607.
User impact if declined: 
  A remote debugger connection will be started for all webapps; users will see
  a notification for each app they launch.
Testing completed (on m-c, etc.):
  This has been on Central for a week.
Risk to taking this patch (and alternatives if risky): 
  This is a simple, low-risk patch that corrects an obvious datatype bug in a
  conditional.
String or IDL/UUID changes made by this patch:
  None.
Attachment #8371139 - Flags: approval-mozilla-aurora?
Attachment #8371139 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.