Closed Bug 793053 Opened 7 years ago Closed 7 years ago

Implement Android product announcements client service

Categories

(Android Background Services Graveyard :: Product Announcements, defect, P1)

ARM
Android
defect

Tracking

(firefox18+ disabled, firefox19 disabled)

RESOLVED FIXED
Tracking Status
firefox18 + disabled
firefox19 --- disabled

People

(Reporter: rnewman, Assigned: rnewman)

References

()

Details

(Whiteboard: [snippets])

Attachments

(6 files, 11 obsolete files)

1.02 KB, patch
gps
: review+
Details | Diff | Splinter Review
1.06 KB, patch
gps
: review+
Details | Diff | Splinter Review
4.83 KB, patch
nalexander
: review+
Details | Diff | Splinter Review
3.27 KB, patch
rnewman
: review+
Details | Diff | Splinter Review
91.05 KB, patch
Details | Diff | Splinter Review
3.26 KB, patch
Details | Diff | Splinter Review
Implementation bug for Android service.
Blocks: 774497
No longer blocks: 774479
Blocks: 793056
We need various hooks into the system to start our notifications service. One of those will be Fennec startup.
No longer blocks: 793056
Version: Firefox 17 → Trunk
Whiteboard: [snippets]
Part-way there.
Attachment #663380 - Attachment is obsolete: true
Remaining work:

* Fix Makefile for Fennec build inclusion of platform.
* Downgrade logging again.
* Possibly adjust Logger to use ThreadLocal log tag, allowing us to log as something other than FxSync. (Or subclass Logger…)
* Stretch alarm interval.
* As many tests as possible. This is really, really hard to write automated tests for!
* Extract real variable for channel (beta etc.)
* Either finish implementing, or rip out, snippet queuing.
* Rework notification code to support earlier Android versions.

* QA steps for:
** Background data on/off
** Network issues/retries
** Display
** Service start on phone restart, Fennec launch, …?
Oh, and:

* Obey backoff headers; other TODOs
* Test User-Agent adding (code is done, just not tested)
Depends on: 795499
Implemented:
* platform and channel inclusion.
* Logging downgrade.
* Alarm interval extension.
* UA tests.
* Both alarm interval and fetch URL can be retrieved from prefs.
  -- Future: add-on to allow easier testing of this feature by
     setting these, and prompting a fetch!
* Notifications now work on pre-11 Android versions.
Attachment #665267 - Attachment is obsolete: true
Attached patch Part 2: snippets service. v1 (obsolete) — Splinter Review
Fix indentation.
Attachment #666152 - Attachment is obsolete: true
Attachment #667649 - Flags: review?(gps)
Worst indentation EVAR.
Attachment #667649 - Attachment is obsolete: true
Attachment #667649 - Flags: review?(gps)
Attachment #667650 - Flags: review?(gps)
Attachment #666151 - Flags: review?(gps)
Comment on attachment 667650 [details] [diff] [review]
Part 0b: add MOZ_UPDATE_CHANNEL to Makefile.in. v3

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

r+ with proper indentation.
Attachment #667650 - Flags: review?(gps) → review+
Attachment #666151 - Flags: review?(gps) → review+
   https://hg.mozilla.org/integration/mozilla-inbound/rev/4417d4fecc1a
   https://hg.mozilla.org/integration/mozilla-inbound/rev/619638eafdc7

Landing the makefile parts to avoid annoyance later.
Whiteboard: [snippets] → [snippets][leave open after merge]
Blocks: 789296
Blocks: 800244
Depends on: 799834
Will be tidied up further by Bug 798043… eventually. I don't want to block on complete perfection.
Attachment #666153 - Attachment is obsolete: true
Attachment #670246 - Flags: review?(nalexander)
Not *quite* ready for review yet, because it's never been tested against a live service. But I have tested fetching, 404s, display, background data off, pref toggling, etc.
Attachment #666154 - Attachment is obsolete: true
This should be a rubber-stamp. We're adding new services, which are put into different fragment files rather than polluting Sync's.

This is the trivial AndroidManifest.xml.in change to include those files, too.
Attachment #670248 - Flags: review?(blassey.bugs)
I'm using Bug 798043 to handle a minimal s/sync/services in mobile/android, so flagging that as a dependency.

blassey, feel free to shunt these reviews to someone else who has free cycles! Thanks.
Depends on: 798043
Note for future QA about verifying background data on ICS: there's no longer a toggle.

Settings > Data usage. Turn on a limit; turn on limit data usage; drag the limits below current data usage; turn off wifi. You'll get a warning that data is no longer available.
Attachment #670248 - Flags: review?(blassey.bugs) → review+
Comment on attachment 670246 [details] [diff] [review]
Part 1: augment GlobalConstants for product announcements. v2

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

r+ after answering questions.

::: mobile/android/base/sync/GlobalConstants.java.in
@@ +15,5 @@
> +  // One of 'beta', 'aurora', 'nightly', 'default'.
> +  // If this is an official build, 'default' means 'release'.
> +  // Otherwise, it means 'dev'.
> +  public static final String MOZ_UPDATE_CHANNEL = "@MOZ_UPDATE_CHANNEL@";
> +#ifdef MOZ_OFFICIAL_BRANDING

Explain to me why we're doing this with an #ifdef rather than introducing @MOZ_OFFICIAL_BRANDING@.

@@ +28,5 @@
> +  public static final String SYNC_VERSION_STRING = SYNC_MAJOR_VERSION + "." +
> +                                                   MOZ_APP_VERSION + "." +
> +                                                   SYNC_MINOR_VERSION;
> +
> +  public static final String SYNC_USER_AGENT = "Firefox AndroidSync " +

Explain why we're not including @MOZ_OFFICIAL_BRANDING@ in the UA.
Attachment #670246 - Flags: review?(nalexander) → review+
Part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/009608ca1b1c

Thanks, chaps.

(In reply to Nick Alexander :nalexander from comment #20)

> Explain to me why we're doing this with an #ifdef rather than introducing
> @MOZ_OFFICIAL_BRANDING@.

Partly because there's not too much win in having a boolean exposed in this way, but I'll file a follow-up so we can get it into preprocess.sh.


> > +  public static final String SYNC_USER_AGENT = "Firefox AndroidSync " +
> 
> Explain why we're not including @MOZ_OFFICIAL_BRANDING@ in the UA.

Firstly, this is the Sync UA, which we don't change lightly, and not in this bug! Secondly, I don't think it offers much value.
I landed Part 3 with empty stub manifest fragments, just so it's not waiting on Part 2, which is blocked on a test service and review.

(Of course, I flipped the numbers around.)

https://hg.mozilla.org/integration/mozilla-inbound/rev/89cb60a08159
Attached patch Main code. v3 (obsolete) — Splinter Review
This is Part 2 (now Part 3) after addressing most of the first run of review comments. We now depend on Bug 801225, which is a lot of general cleanup to allow our logging and constants to live alongside Android Sync.
Attachment #670247 - Attachment is obsolete: true
Depends on: 801225
Summary: Implement Android Snippets client service → Implement Android product announcements client service
Remaining items for this:

* Finish last of Nick's review comments.
* Test SD card and boot events.
* Test against live server (JR's just wrapping that up, I think).
* Sec review (today).
* Obtain a final URI to bake into the product.
* Test URI pref.
* Decide whether we would like the client to respect 301s by setting the server URI pref, which allows us some flexibility there.
I'm working on an add-on to allow QA of this feature -- switching the server URL and the fetch interval.

WIP:

https://github.com/mozilla-services/product-announcements-test-addon
Hand-verified startup intent:

10-23 22:29:01.256 I/GeckoAnnounce( 3659): fennec_rnewman :: AnnounceBrSvc :: Broadcast onReceive. Intent is android.intent.action.BOOT_COMPLETED
10-23 22:29:01.256 I/GeckoAnnounce( 3659): fennec_rnewman :: AnnounceBrSvc :: Asking Fennec.
10-23 22:29:01.266 I/ActivityManager( 2109): Start proc app.processName for broadcast hostingNameStr: pid=3676 uid=10150 gids={1015, 1023, 1006, 3001, 3002, 3003}
10-23 22:29:01.271 I/dalvikvm( 3676): Turning on JNI app bug workarounds for target SDK version 4...
10-23 22:29:01.276 D/GeckoPreferences( 3659): Broadcast: org.mozilla.gecko.ANNOUNCEMENTS_PREF, android.not_a_preference.privacy.announcements.enabled, GeckoApp, true
10-23 22:29:01.281 I/GeckoAnnounce( 3659): fennec_rnewman :: AnnounceBrSvc :: Asked Fennec.
Comment on attachment 670246 [details] [diff] [review]
Part 1: augment GlobalConstants for product announcements. v2

Might as well get as much as we can into Aurora while we wait for Part 3 to finish.
Attachment #670246 - Flags: approval-mozilla-aurora?
This is part 2 as landed, so I can request Aurora approval for the actual code.

Just pre-landing some stuff to have a smaller chunk to do later. Risk-averse!

No strings, no UUIDs; this patch is essentially including three empty files = no-op.
Attachment #670248 - Attachment is obsolete: true
Attachment #675342 - Flags: review+
Attachment #675342 - Flags: approval-mozilla-aurora?
Attached patch Main code. v4 (obsolete) — Splinter Review
Now with lots of little tweaks, review comments all addressed, backoff fuzzed.
Attachment #671035 - Attachment is obsolete: true
Attachment #675398 - Flags: review?(nalexander)
Should we opt for this (because we don't have a final URI and a place to test), here's a pref-off.
Attachment #675400 - Flags: review?(nalexander)
   https://hg.mozilla.org/integration/mozilla-inbound/rev/717cd6ee20fc
   https://hg.mozilla.org/integration/mozilla-inbound/rev/1330fdc14cb0

I'll upload updated patches momentarily.

Note that this is preffed off (actually a build-time off switch), and the server URL is:

  https://campaigns.services.mozilla.com/announce/

which means requests will be for

  https://campaigns.services.mozilla.com/announce/1/android/
Whiteboard: [snippets][leave open after merge] → [snippets]
Target Milestone: --- → Firefox 19
[Approval Request Comment]

User impact if declined: 
  No popups in 18.

Testing completed (on m-c, etc.): 
  About to begin baking on m-c. Note that at this point the feature isn't turned on; I'll have it turned on in Nightly for a while before requesting uplift for the patch to turn it on!

Risk to taking this patch (and alternatives if risky): 
  Product announcements feature doesn't land in Fx18.

String or UUID changes made by this patch: 
  None.
Attachment #675398 - Attachment is obsolete: true
Attachment #675398 - Flags: review?(nalexander)
Attachment #675768 - Flags: approval-mozilla-aurora?
And here's the pref-off part.
Attachment #675400 - Attachment is obsolete: true
Attachment #675400 - Flags: review?(nalexander)
Attachment #675770 - Flags: approval-mozilla-aurora?
Marking this as disabled in fx19.
Blocks: 806024
Blocks: 806634
Attachment #670246 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #675342 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #675768 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #675770 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Component: General → Android: Product Announcements
Product: Firefox for Android → Mozilla Services
Target Milestone: Firefox 19 → ---
Version: Trunk → unspecified
Tracy: steps for testing are below. I assume you're testing Beta.

* adb shell

  setprop log.tag.GeckoAnnounce VERBOSE

* Install Firefox.
* In another terminal, `adb logcat -v time | fgrep Announce`.
* Install setannounceprefs.xpi from <https://github.com/mozilla-services/product-announcements-test-addon/downloads>. You can do this with `adb push`:

  adb push setannounceprefs.xpi /sdcard/setannounceprefs.xpi
  adb shell am start -a android.intent.action.VIEW \
                     -c android.intent.category.DEFAULT \
                     -d file:///mnt/sdcard/setannounceprefs.xpi \
                     -n org.mozilla.firefox_beta/.App

* Choose "Install".
* Menu > "Set announcements prefs".

* You should see log output like

11-29 18:38:50.575 D/GeckoSetPrefs(18751): Set prefs to http://aws-campaign-manager-dev.services.mozilla.com/announce/, 15000
11-29 18:38:50.580 D/GeckoPreferences(18751): Broadcast: org.mozilla.gecko.ANNOUNCEMENTS_PREF, android.not_a_preference.privacy.announcements.enabled, GeckoApp, true
11-29 18:38:50.585 D/GeckoSetPrefs(18751): Announcement broadcast.
11-29 18:38:50.615 D/GeckoAnnounce(18751): firefox_beta :: AnnounceBrSvc :: Broadcast onReceive. Intent is org.mozilla.gecko.ANNOUNCEMENTS_PREF
11-29 18:38:50.615 D/GeckoAnnounce(18751): firefox_beta :: AnnounceBrSvc :: GeckoApp/android.not_a_preference.privacy.announcements.enabled = true
11-29 18:38:50.615 I/GeckoAnnounce(18751): firefox_beta :: AnnounceBrSvc :: Registering announcements broadcast receiver...
11-29 18:38:50.620 I/GeckoAnnounce(18751): firefox_beta :: AnnounceBrSvc :: Setting inexact repeating alarm for interval 15000
11-29 18:38:50.650 D/GeckoAnnounce(18751): firefox_beta :: AnnounceService :: Creating AnnouncementsService.
11-29 18:38:50.655 D/GeckoAnnounce(18751): firefox_beta :: AnnounceService :: Running AnnouncementsService.
11-29 18:38:50.655 D/GeckoAnnounce(18751): firefox_beta :: AnnounceFetch :: Fetching announcements. Last launch: 1354242959919; now: 1354243130661
11-29 18:38:50.655 D/GeckoAnnounce(18751): firefox_beta :: AnnounceFetch :: Fetch URI: idle for 0 days.
11-29 18:38:50.655 I/GeckoAnnounce(18751): firefox_beta :: AnnounceFetch :: Fetching announcements from http://aws-campaign-manager-dev.services.mozilla.com/announce/1/android/beta/18.0/armeabi-v7a?idle=0
11-29 18:38:50.655 D/GeckoAnnounce(18751): firefox_beta :: BaseResource :: HTTP GET http://aws-campaign-manager-dev.services.mozilla.com/announce/1/android/beta/18.0/armeabi-v7a?idle=0
11-29 18:38:51.140 D/GeckoAnnounce(18751): firefox_beta :: BaseResource :: Response: HTTP/1.1 500 Internal Server Error
11-29 18:38:51.140 D/GeckoAnnounce(18751): firefox_beta :: AnnounceFetchRD :: Got announcements response: 500
11-29 18:38:51.145 W/GeckoAnnounce(18751): firefox_beta :: AnnounceFetchRD :: Server issue: Internal Server Error
11-29 18:38:51.145 W/GeckoAnnounce(18751): 
11-29 18:38:51.145 W/GeckoAnnounce(18751): The server encountered an unexpected internal server error
11-29 18:38:51.145 W/GeckoAnnounce(18751): 
11-29 18:38:51.145 W/GeckoAnnounce(18751): (generated by waitress)
11-29 18:38:51.145 I/GeckoAnnounce(18751): firefox_beta :: AnnounceService :: Got retry after: -1
11-29 18:38:51.145 D/GeckoAnnounce(18751): firefox_beta :: AnnounceService :: Fuzzed backoff: 111ms.


When the service is working, you should see announcements :)
Note that every time you relaunch Firefox you should tap "Set Announcements." The fetches should continue to occur every 15 seconds, even if you quit Firefox. If they don't, please let me know!
Note also that if you force-stop the app, Android will also terminate all of its services, and we won't be able to message them. That's part of Android's design.

If you merely close the activity (hit the Home button), you should see the fetches continue in the background.
Depends on: 800071
Depends on: 823679
Component: Android: Product Announcements → Product Announcements
Product: Mozilla Services → Android Background Services
You need to log in before you can comment on or make changes to this bug.