1330151 - Recovering lost Push architecture docs

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[:Grisha Kruglov]

Bug 1207714 had nice architecture docs of fennec's implementation of push that somehow got lost in mozreview and didn't make it into the tree. See https://reviewboard.mozilla.org/r/36721/diff/1#index_header.

Comment 1

[:Grisha Kruglov]

Attachment: Bug 1330151 - Push Architecture docs recovered from Bug 1207714

Nick's original docs copy-pasted verbatim.

Review commit: https://reviewboard.mozilla.org/r/103702/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103702/

Comment 2

[:Grisha Kruglov]

Attachment: Bug 1330151 - Adjust docs slightly to better reflect current implementation

Review commit: https://reviewboard.mozilla.org/r/103704/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103704/

Comment 3

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8825577 [details]
Bug 1330151 - Push Architecture docs recovered from Bug 1207714

https://reviewboard.mozilla.org/r/103702/#review104360

::: mobile/android/docs/push.rst:66
(Diff revision 1)
> +Gecko profile) in order to service incoming push messages.  Right now, if a push
> +message is received and Gecko is not running (with the correct profile) to
> +service the message, it will be dropped on the floor (with
> +helpful-but-unsatisfying diagnostic messages).  In particular, **neither the
> +sending application server nor the receiving web application are informed of the
> +failed delivery at any time!** (We may, at some point, choose to queue messages

This queueing has been implemented.  So fold the following line into the previous paragraph and delete the outdated paragraph.  Line:

`Fennec will start Gecko (more accurately, a particular Gecko profile) in order to service incoming push messages.`

::: mobile/android/docs/push.rst:93
(Diff revision 1)
> +valuable and difficult.  Therefore, we add the following requirement:
> +
> +4) Gecko must own the push configuration details where appropriate and possible.
> +
> +We explicitly do not care to support push messages across multiple processes.
> +This means that we **do not support the Push API in APK-based Web Apps that run

This is no longer relevant; these web apps are no longer supported.  Just drop these two lines.  (The comment about multiple processes is still relevant, and matters more in a post-e10s-on-Android world.)

::: mobile/android/docs/push.rst:125
(Diff revision 1)
> +The second mapping is a one-to-many mapping from push registrations to
> +`PushSubscription` instances.  A push subscription corresponds to a unique push
> +message channel from the autopush server to Fennec.  Each `PushSubscription`
> +includes:
> +
> +* a Fennec service identifier, like "webpush" or "Sync" or "Firefox Accounts";

I think this is now `"webpush" or "fxa"`: see https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/push/PushService.java#56.

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8825578 [details]
Bug 1330151 - Adjust docs slightly to better reflect current implementation

https://reviewboard.mozilla.org/r/103704/#review104364

::: mobile/android/docs/push.rst:60
(Diff revision 1)
>  
>  The Fennec Push implementation is designed to address the following technical
>  challenge: **GCM events, including incoming push messages, can occur when Gecko
> -is not running**.
> -
> -In the future, Fennec will be able to start Gecko (more accurately, a particular
> +is not running**. In case of an incoming push message, if Gecko is not running
> +Fennec will request startup of necessary Gecko services and queue incoming
> +push messages in the meantime. Once services are running, messages are sent over.

Oh, snap!  Fold my additional notes in here.  Well done!

Comment 5

[:Grisha Kruglov]

Comment on attachment 8825578 [details]
Bug 1330151 - Adjust docs slightly to better reflect current implementation

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/103704/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4910b42e2042
Push Architecture docs recovered from Bug 1207714 r=nalexander
https://hg.mozilla.org/integration/autoland/rev/9e02d11fb965
Adjust docs slightly to better reflect current implementation r=nalexander

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4910b42e2042
https://hg.mozilla.org/mozilla-central/rev/9e02d11fb965