Open Bug 1242213 Opened 6 years ago Updated 2 years ago

Remove unused <activity-alias> definitions in Fennec manifest and use android:packageName="org.mozilla.gecko"

Categories

(Firefox Build System :: Android Studio and Gradle Integration, defect)

defect
Not set
normal

Tracking

(firefox47 fixed)

REOPENED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(4 files, 5 obsolete files)

The Android toolchain has moved forward, and now properly supports multiple APKs with distinct Android package names generated from a single AndroidManifest.xml file.  The way this works is that the manifest defines a single android:packageName and tooling defines the APK package.  For us, we will have android:packageName="org.mozilla.gecko".  Gradle uses applicationId to determine the APK package (for us, org.mozilla.fennec and co.).  This ticket tracks using the --rename-package flag to aapt.

In order to accommodate this, we should also remove the <activity-alias> definitions in the manifest.  They were always a precaution, and I'm now certain they're not needed.
Component: Build Config → Build Config & IDE Support
Product: Core → Firefox for Android
This <activity> and <activity-alias> support old-style homescreen
shortcuts to old-style Webapps.  Such shortcuts must have been
produced at least 18 months ago, and pre-date the new-style synthetic
APK Webapps entirely.  New-style APK Webapps are slated to be removed
from the product entirely, and there's no reason to keep their even
less viable predecessor around.

Telemetry from http://mzl.la/23kXGV5 shows that there were no launches
of webapps (old-style or new-style) for Fennec 43 release.  Telemetry
from http://mzl.la/23kXFAs shows that there were 40.7k launches of
webapps (again, old-style or new-style) for Fennec 44 beta (of 39.0M
total -- for 0.1% total).  We cannot distinguish old-style from
new-style, but it is safe to assume it's a tiny proportion.

Users with such homescreen shortcuts will see a bogus "App isn't
installed" message and need to delete and re-create their shortcut in
some way.

The org.mozilla.gecko.Webapp class cannot be removed until new-style
APK Webapps are removed.

Review commit: https://reviewboard.mozilla.org/r/32139/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32139/
Attachment #8711385 - Flags: review?(mark.finkle)
Way back in Fennec 33 (Bug 929865, see
https://hg.mozilla.org/mozilla-central/rev/a4f39c9db1d9) we replaced
org.mozilla.gecko.App with org.mozilla.gecko.BrowserApp and introduced
the .App <activity-alias>.  Per the entry for android:name of
http://developer.android.com/guide/topics/manifest/activity-element.html,
.App translates to $ANDROID_PACKAGE_NAME.App.  We haven't referenced
an Activity qualified with a non-org.mozilla.gecko *class* name (for
example, from bookmark shortcuts) since well before Fennec 33, so this
probably never did what it was intended to do: we wanted to redirect
org.mozilla.gecko.App to org.mozilla.gecko.BrowserApp, but it instead
has been redirecting org.mozilla.fennec.App to
org.mozilla.gecko.BrowserApp.  I don't think we've been referring to
$ANDROID_PACKAGE_NAME.App since well before Fennec 29, if ever.

The <activity-alias> has been servicing essentially all
<intent-filter> invocations of Fennec by passing them directly to
org.mozilla.gecko.BrowserApp.

This pushes a long programme of simplification forward.  Fallout might
look like very old homescreen shortcuts failing to launch, but I'm
quite confident that won't actually happen.

Review commit: https://reviewboard.mozilla.org/r/32141/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32141/
Attachment #8711386 - Flags: review?(mark.finkle)
I have manually verified that this results in a byte-identical
gecko.ap_.  This is because after the earlier patches there are no
definitions (or aliases) that are package-local (like .App or
.Webapp), which are the only things (other than the Android package
name) that get rewritten by --rename-manifest-package.

Review commit: https://reviewboard.mozilla.org/r/32143/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32143/
Attachment #8711387 - Flags: review?(mark.finkle)
Depends on: 1237755
Comment on attachment 8711385 [details]
MozReview Request: Bug 1242213 - Part 1: Remove old-style Webapp entry-point. r?mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32139/diff/1-2/
Comment on attachment 8711386 [details]
MozReview Request: Bug 1242213 - Part 2: Fold App <activity-alias> into BrowserApp <activity>. r?mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32141/diff/1-2/
Comment on attachment 8711387 [details]
MozReview Request: Bug 1242213 - Part 3: Use android:packageName="org.mozilla.gecko" and --rename-manifest-package. r?mfinkle

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/32143/diff/1-2/
Comment on attachment 8711385 [details]
MozReview Request: Bug 1242213 - Part 1: Remove old-style Webapp entry-point. r?mfinkle

myk: I'd just like to make you aware of this deletion.
Attachment #8711385 - Flags: feedback?(myk)
Comment on attachment 8711385 [details]
MozReview Request: Bug 1242213 - Part 1: Remove old-style Webapp entry-point. r?mfinkle

> myk: I'd just like to make you aware of this deletion.

Makes sense, thanks for the heads-up!
Attachment #8711385 - Flags: feedback?(myk) → feedback+
Attachment #8711385 - Flags: review?(mark.finkle) → review+
Comment on attachment 8711385 [details]
MozReview Request: Bug 1242213 - Part 1: Remove old-style Webapp entry-point. r?mfinkle

https://reviewboard.mozilla.org/r/32139/#review29279

LGTM
Attachment #8711386 - Flags: review?(mark.finkle) → review+
Comment on attachment 8711386 [details]
MozReview Request: Bug 1242213 - Part 2: Fold App <activity-alias> into BrowserApp <activity>. r?mfinkle

https://reviewboard.mozilla.org/r/32141/#review29281

LGTM, assuming this doesn't break anything.
Comment on attachment 8711387 [details]
MozReview Request: Bug 1242213 - Part 3: Use android:packageName="org.mozilla.gecko" and --rename-manifest-package. r?mfinkle

https://reviewboard.mozilla.org/r/32143/#review29283

Onward!
Attachment #8711387 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/fx-team/rev/2b77aa9048009d076e4dd54a6890657815432b03
Bug 1242213 - Part 0: Use org.mozilla.gecko.BrowserApp instead of .App. r=me,gbrown,bc

https://hg.mozilla.org/integration/fx-team/rev/b517a7f4296a430ac91f704eaa8f728f639e6e40
Bug 1242213 - Part 1: Remove old-style Webapp entry-point. r=mfinkle f=myk

https://hg.mozilla.org/integration/fx-team/rev/92227c8d185ba25e540d7f3ecd82c6d9203e6e54
Bug 1242213 - Part 2: Fold App <activity-alias> into BrowserApp <activity>. r=mfinkle

https://hg.mozilla.org/integration/fx-team/rev/10dfe5e3ded1621a7ef777ddbc54e5f80696c3d6
Bug 1242213 - Part 3: Use android:packageName="org.mozilla.gecko" and --rename-manifest-package. r=mfinkle
(In reply to Nick Alexander :nalexander from comment #22)
> https://hg.mozilla.org/integration/fx-team/rev/
> 28985a37c1055f18c421d90767475b71ca45ebb5
> Backed out changeset 10dfe5e3ded1 (bug 1242213)

Sadly, Part 3 didn't quite work -- it caused errors fetching android.resource://pkg/drawable/... resources from Picasso, and also there were some small usages of org.mozilla.fennec that needed to be changed to org.mozilla.gecko.

Unfortunately, Part 3 is required by subsequent Gradle changes.  I'll be working on addressing this, or backing out those Gradle changes, over the weekend.
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Yep, just merged that to m-c, and then learned that the backout I didn't even see is what fixed the 25 hours of permaorange we'd been misstarring.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Phil Ringnalda (:philor) from comment #25)
> Yep, just merged that to m-c, and then learned that the backout I didn't
> even see is what fixed the 25 hours of permaorange we'd been misstarring.

Thanks philor.  This was tricky, 'cuz the permaorange failure was identical to the actual (different!) intermittent orange.  Le sigh.
Attachment #8716692 - Attachment is obsolete: true
Attachment #8716694 - Attachment is obsolete: true
Attachment #8716696 - Attachment is obsolete: true
Attachment #8717171 - Attachment is obsolete: true
Attachment #8717178 - Attachment is obsolete: true
Blocks: 1247637
Depends on: 1248642
Blocks: 1256615
Depends on: 1256974
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 47 → mozilla47
You need to log in before you can comment on or make changes to this bug.