Closed Bug 1242213 Opened 9 years ago Closed 1 year 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

Tracking

(firefox47 fixed)

RESOLVED WONTFIX
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)
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.
Attached patch Backed out changeset 10dfe5e3ded1 (), (obsolete) (deleted) — Splinter Review
Attached patch Backed out changeset 10dfe5e3ded1 (), (obsolete) (deleted) — Splinter Review
Attachment #8716692 - Attachment is obsolete: true
Attached patch Backed out changeset 10dfe5e3ded1 (), (obsolete) (deleted) — Splinter Review
Attachment #8716694 - Attachment is obsolete: true
Attached patch Backed out changeset 10dfe5e3ded1 (), (obsolete) (deleted) — Splinter Review
Attachment #8716696 - Attachment is obsolete: true
Attached patch Backed out changeset 10dfe5e3ded1 (), (obsolete) (deleted) — Splinter Review
Attachment #8717171 - Attachment is obsolete: true
Attachment #8717178 - Attachment is obsolete: true
Depends on: 1248642
Depends on: 1256974
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 47 → mozilla47
Severity: normal → S3
Priority: -- → P1

This popped up on our team's triage board - I'm going to close this as won't fix though since it's an 8 year old bug from fennec days.

Status: REOPENED → RESOLVED
Closed: 9 years ago1 year ago
Priority: P1 → --
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: