669424 - get rid of shared ID

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

According to jmaher we don't need it and its been a big headache all along.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 2

[Doug Turner (:dougt)]

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.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

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

[Mike Taylor [:bear]]

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

Comment 5

[Aki Sasaki (not active)]

Afaik without the shared ID everything tegra-based will burn.

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

try build ran green http://tbpl.mozilla.org/?tree=Try&rev=cd15117af00d

Bob, feedback ping?

Comment 7

[Bob Moss :bmoss]

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

Comment 8

[Bob Moss :bmoss]

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.

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/32135b36a00e

Comment 10

[Matt Brubeck (:mbrubeck)]

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

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

we can at least get rid of this for private builds and eliminate one class of problems the shared id brings

Comment 12

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch to get rid of shared id in private builds

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch to get rid of shared id in private builds

forgot the makefile change

Comment 14

[Matt Brubeck (: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
       >

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

review nits are pretty much a complete re-write, asking for a re-review

Comment 16

[Matt Brubeck (: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).

Comment 17

[Brad Lassey [:blassey] (use needinfo?)]

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

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/386bc44b98ff

Comment 19

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/386bc44b98ff