get rid of shared ID

RESOLVED FIXED in mozilla8

Status

()

Core
Widget: Android
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: blassey, Assigned: blassey)

Tracking

Trunk
mozilla8
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

According to jmaher we don't need it and its been a big headache all along.
Created attachment 544024 [details] [diff] [review]
patch
Attachment #544024 - Flags: review?(doug.turner)
Attachment #544024 - Flags: feedback?(bmoss)

Comment 2

6 years ago
Comment on attachment 544024 [details] [diff] [review]
patch

this patch does what it is suppose to.  lets make sure that bmoss is happy with this change and that it doesn't break anything.
Attachment #544024 - Flags: review?(doug.turner) → review+
to clarify, the shared ID is responsible for allowing us access to:
/data/data/org.mozilla.fennec/ (or appname) directory. 

We can still launch and kill the process as well as use our profile on the sdcard.  For Talos, we access application.ini, since we have a remote script for this, we can edit that to not require access.  We will need to examine the buildbot scripts to make sure nothing else accesses this directory.

Comment 4

6 years ago
Currently the install apk build step pushes application.ini to that directory

Comment 5

6 years ago
Afaik without the shared ID everything tegra-based will burn.
(Assignee)

Updated

6 years ago
Depends on: 669968
try build ran green http://tbpl.mozilla.org/?tree=Try&rev=cd15117af00d

Bob, feedback ping?

Comment 7

6 years ago
Sorry, let me gather the info and will provide feedback shortly.

Comment 8

6 years ago
Ok, so afaict releng and ateam patches have landed to support this. So I recommend that we land it Monday or Tuesday of next week so that we have all hands available to debug and/or back out if we've missed something.

Updated

6 years ago
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/32135b36a00e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
OS: Linux → Android
Hardware: x86_64 → All
Depends on: 673168
Backed out this change because it caused startup failures after updating.  It looks like we cannot change or remove the sharedUserId without breaking things.

http://hg.mozilla.org/mozilla-central/rev/99d2fda67471
Resolution: FIXED → WONTFIX
we can at least get rid of this for private builds and eliminate one class of problems the shared id brings
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Created attachment 550192 [details] [diff] [review]
patch to get rid of shared id in private builds
Assignee: nobody → blassey.bugs
Attachment #544024 - Attachment is obsolete: true
Status: REOPENED → NEW
Attachment #550192 - Flags: review?(mbrubeck)
Attachment #544024 - Flags: feedback?(bmoss)
Created attachment 550200 [details] [diff] [review]
patch to get rid of shared id in private builds

forgot the makefile change
Attachment #550192 - Attachment is obsolete: true
Attachment #550200 - Flags: review?(mbrubeck)
Attachment #550192 - Flags: review?(mbrubeck)
Comment on attachment 550200 [details] [diff] [review]
patch to get rid of shared id in private builds

Yes, we should definitely remove this from unofficial builds.  We'll have to announce to developers that they'll need to do clean installs of their local builds after this is checked in.

>+#if UNOFFICIAL_BUILD
>+      >
>+#else
>       android:sharedUserId="@MOZ_ANDROID_SHARED_ID@">
>+#endif

Nitpick - I would prefer it like this:

 #ifdef MOZ_ANDROID_SHARED_ID
       android:sharedUserId="@MOZ_ANDROID_SHARED_ID@"
 #endif
       >
Attachment #550200 - Flags: review?(mbrubeck) → review+
Created attachment 551810 [details] [diff] [review]
patch

review nits are pretty much a complete re-write, asking for a re-review
Attachment #550200 - Attachment is obsolete: true
Attachment #551810 - Flags: review?(mbrubeck)
Comment on attachment 551810 [details] [diff] [review]
patch

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

r=mbrubeck with one change noted below.

::: embedding/android/Makefile.in
@@ +111,1 @@
>  DEFINES += -DMOZ_ANDROID_SHARED_ID="org.mozilla.fennec.sharedID"

I think you're missing a case for ANDROID_PACKAGE_NAME == org.mozilla.fennec (for official Nightly builds).
Attachment #551810 - Flags: review?(mbrubeck) → review+
(In reply to Matt Brubeck (:mbrubeck) from comment #16)
> >  DEFINES += -DMOZ_ANDROID_SHARED_ID="org.mozilla.fennec.sharedID"
> 
> I think you're missing a case for ANDROID_PACKAGE_NAME == org.mozilla.fennec
> (for official Nightly builds).

and that's why we do code review...
http://hg.mozilla.org/integration/mozilla-inbound/rev/386bc44b98ff
http://hg.mozilla.org/mozilla-central/rev/386bc44b98ff
Status: NEW → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.