Last Comment Bug 669424 - get rid of shared ID
: get rid of shared ID
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Android (show other bugs)
: Trunk
: All Android
: -- normal (vote)
: mozilla8
Assigned To: Brad Lassey [:blassey] (use needinfo?)
:
Mentors:
Depends on: 669968 673168
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-05 12:34 PDT by Brad Lassey [:blassey] (use needinfo?)
Modified: 2011-08-10 08:32 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.70 KB, patch)
2011-07-05 12:40 PDT, Brad Lassey [:blassey] (use needinfo?)
doug.turner: review+
Details | Diff | Splinter Review
patch to get rid of shared id in private builds (756 bytes, patch)
2011-08-02 14:03 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
patch to get rid of shared id in private builds (1.36 KB, patch)
2011-08-02 14:20 PDT, Brad Lassey [:blassey] (use needinfo?)
mbrubeck: review+
Details | Diff | Splinter Review
patch (2.02 KB, patch)
2011-08-09 10:02 PDT, Brad Lassey [:blassey] (use needinfo?)
mbrubeck: review+
Details | Diff | Splinter Review

Description Brad Lassey [:blassey] (use needinfo?) 2011-07-05 12:34:40 PDT
According to jmaher we don't need it and its been a big headache all along.
Comment 1 Brad Lassey [:blassey] (use needinfo?) 2011-07-05 12:40:01 PDT
Created attachment 544024 [details] [diff] [review]
patch
Comment 2 Doug Turner (:dougt) 2011-07-05 12:59:50 PDT
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) 2011-07-05 13:04:45 PDT
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] 2011-07-05 13:06:47 PDT
Currently the install apk build step pushes application.ini to that directory
Comment 5 Aki Sasaki [:aki] 2011-07-05 13:18:06 PDT
Afaik without the shared ID everything tegra-based will burn.
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2011-07-14 15:53:18 PDT
try build ran green http://tbpl.mozilla.org/?tree=Try&rev=cd15117af00d

Bob, feedback ping?
Comment 7 Bob Moss :bmoss 2011-07-15 13:08:26 PDT
Sorry, let me gather the info and will provide feedback shortly.
Comment 8 Bob Moss :bmoss 2011-07-15 13:20:27 PDT
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] 2011-07-19 08:06:26 PDT
http://hg.mozilla.org/mozilla-central/rev/32135b36a00e
Comment 10 Matt Brubeck (:mbrubeck) 2011-07-21 14:18:54 PDT
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?) 2011-08-02 14:02:07 PDT
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?) 2011-08-02 14:03:02 PDT
Created attachment 550192 [details] [diff] [review]
patch to get rid of shared id in private builds
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2011-08-02 14:20:19 PDT
Created attachment 550200 [details] [diff] [review]
patch to get rid of shared id in private builds

forgot the makefile change
Comment 14 Matt Brubeck (:mbrubeck) 2011-08-02 14:39:26 PDT
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?) 2011-08-09 10:02:36 PDT
Created attachment 551810 [details] [diff] [review]
patch

review nits are pretty much a complete re-write, asking for a re-review
Comment 16 Matt Brubeck (:mbrubeck) 2011-08-09 10:19:26 PDT
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?) 2011-08-09 10:32:41 PDT
(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?) 2011-08-09 22:13:01 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/386bc44b98ff

Note You need to log in before you can comment on or make changes to this bug.