Closed Bug 1235050 Opened 4 years ago Closed 4 years ago

Simple Push doesn't work anymore

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(firefox46 fixed)

RESOLVED FIXED
2.6 S5 - 1/15
Tracking Status
firefox46 --- fixed

People

(Reporter: jovan.gerodetti, Assigned: gerard-majax)

References

Details

(Keywords: regression, Whiteboard: [systemsfe])

Attachments

(4 files, 1 obsolete file)

Today I discovered that Simple Push doesn't work anymore on latest nightly. (Build ID: 20151223150227)

If I run something as simple as this:

var x= navigator.push.register()
x.onsuccess = e => console.log(e);
x.onerror = e => console.log(e);

nothing will happen. There is not even an error. 
I hope someone can investigate this.
Simple Push is an important feature.
Is it possible it is the same as bug 1174420 ?
At least I see the same behavior as in bug 1174420 comment 0. Naoki, I fear this should be smoketest blocker

> alex@portable-alex:~/codaz/Mozilla/b2g/devices/XperiaZ3c/B2G/objdir-gecko/dist/b2g/omni$ grep -R 'PushServiceLauncher' 
> components/components.manifest:component {4b8caa3b-3c58-4f3c-a7f5-7bd9cb24c11d} PushServiceLauncher.js
> components/components.manifest:category app-startup PushServiceLauncher @mozilla.org/push/ServiceLauncher;1
> alex@portable-alex:~/codaz/Mozilla/b2g/devices/XperiaZ3c/B2G/objdir-gecko/dist/b2g/omni$ find . |grep PushServiceLauncher
> alex@portable-alex:~/codaz/Mozilla/b2g/devices/XperiaZ3c/B2G/objdir-gecko/dist/b2g/omni$ find . |grep PushService
> ./modules/PushService.jsm
> alex@portable-alex:~/codaz/Mozilla/b2g/devices/XperiaZ3c/B2G/objdir-gecko/dist/b2g/omni$
Flags: needinfo?(nhirata.bugzilla)
There was some packaging change in bug 1189998: https://hg.mozilla.org/mozilla-central/diff/f821dec83eed/b2g/installer/package-manifest.in#l650

PushServiceLauncher.js is not packaged anymore. There is no dom/simplepush/PushComponents.js

I don't know if that is expected or not. But this is breaking a major feature on B2G: applications like Telegram and others will not be able to send push to users ...
Blocks: 1189998
Flags: needinfo?(kcambridge)
Flags: needinfo?(cbook)
Keywords: regression
Given that dom/simplepush/PushComponents.js does not exists but that dom/simplepush/PushServiceLauncher.js exists, I am wondering if the condition should not be inverted ...
(In reply to Alexandre LISSY :gerard-majax from comment #3)
> There was some packaging change in bug 1189998:
> https://hg.mozilla.org/mozilla-central/diff/f821dec83eed/b2g/installer/
> package-manifest.in#l650
> 
> PushServiceLauncher.js is not packaged anymore. There is no
> dom/simplepush/PushComponents.js
> 
> I don't know if that is expected or not. But this is breaking a major
> feature on B2G: applications like Telegram and others will not be able to
> send push to users ...

yes this is how I discovered it. Telegram went totally unreliable since I did the upgrade to nightly.
Assignee: nobody → lissyx+mozillians
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8701834 [details] [diff] [review]
Fix SimplePush packaging on B2G r=...

Martin, would you mind reviewing this? Looks like there is absolutely no testing at all of dom/simplepush/

Given that I know Naoki is trying to push FOTA for Flame and Z3c, if that patch is good for you, I would suggest we land it as is and we follow up with a mochitest. This has already regressed twice exactly for the same reason.
Attachment #8701834 - Flags: review?(martin.thomson)
Comment on attachment 8701834 [details] [diff] [review]
Fix SimplePush packaging on B2G r=...

No review over thge weekend, I'll send a new patch with tests tomorrow
Attachment #8701834 - Flags: review?(martin.thomson)
Ack.  We already sent out the T2M our flame build.  :(

I agree. We don't have constant testing for push notifications.
ktucker, jlorenzo and I will talk about setting up some tests for this when we all get back ( Jan 4th ).

If you get this patch in on monday, I'll retest the new build on tuesday.
Flags: needinfo?(ktucker)
Flags: needinfo?(jlorenzo)
Attachment #8701834 - Attachment is obsolete: true
Testing is not possible because all emulator ics fails with:
>  [taskcluster-vcs:error] run end (with error) NOT RETRYING!: tar -x -z -C /home/worker/workspace/B2G -f /home/worker/.tc-vcs-repo/sources/git.mozilla.org/b2g/fake-dalvik/master.tar.gz
android has issues here:
[taskcluster-vcs:error] Artifact "public/github.com/mozilla/fennec-distribution-sample.tar.gz" not found for task ID undefined.  This could be caused by the artifact not being created or being marked as expired.


something wonky is going on- either these try pushes are messed up or we should turn off everything until this is fixed.
(In reply to Alexandre LISSY :gerard-majax from comment #15)
> cancelled the previous and triggered new ones:
>  - all: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f18bc678e39
>  - fix with tests:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=1c4e2791e2c8
>  - tests only:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=3027b45bc403

lesson learnt: do not use subsuite, it's not a way to tag :)
(In reply to Alexandre LISSY :gerard-majax from comment #18)
> test only: 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6b19851d515

And we have two expected timeouts: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6b19851d515&selectedJob=14913301

> fix inside:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=64f93026a1dd

And there it passes https://treeherder.mozilla.org/#/jobs?repo=try&revision=64f93026a1dd&selectedJob=14913335

Looks like there is something ongoing with a crash. Hopefully it's just addPermission that needs reload().
Gah, this isn't the first time I broke Simple Push packaging on B2G. :-( Thanks for adding the mochitests. Does the subsuite need to be whitelisted in https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/testing/mochitest/runtests.py#2252?
Flags: needinfo?(kcambridge)
(In reply to Alexandre LISSY :gerard-majax from comment #20)
> fix inside:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c396c0f14d1
> tests only:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5db3e12f4b6

Failing as expected with timeout without the fix :)

(In reply to Kit Cambridge [:kitcambridge] from comment #21)
> Gah, this isn't the first time I broke Simple Push packaging on B2G. :-(
> Thanks for adding the mochitests. Does the subsuite need to be whitelisted
> in
> https://dxr.mozilla.org/mozilla-central/rev/
> 388bdc46ba51ee31da8b8abe977e0ca38d117434/testing/mochitest/runtests.py#2252?

No it looks like we do not need that !
Attachment #8702417 - Flags: review?(kcambridge)
Attachment #8702418 - Flags: review?(kcambridge)
Attachment #8702417 - Flags: review?(kcambridge) → review+
Attachment #8702418 - Flags: review?(kcambridge) → review+
Flags: needinfo?(cbook)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1db8e4432568
https://hg.mozilla.org/mozilla-central/rev/29aa734ee232
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Will this be in tomorrows build?
Yes, there should be a 3 pm PDT build that contains this fix for flame-latest.  It should also be in tomorrow's build.
To note, I'm not sure if this will go in the partial FOTA for the flame device.  I think Alexandre might know better.  I was referring to at least the full build.
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #9)
> I agree. We don't have constant testing for push notifications.
> ktucker, jlorenzo and I will talk about setting up some tests for this when
> we all get back ( Jan 4th ).

Regarding automation, the mochitests Alexandre added are the best approach to cover most of the case, IMO. I'm not familiar enough with the API to see if we're missing other use cases, though.

We could also add an end-to-end test to make sure one notification pops up correctly when emitted by an actual server.
Flags: needinfo?(jlorenzo)
Whiteboard: [systemsfe]
Target Milestone: --- → 2.6 S5 - 1/15
seems like AdamA verified that it is working on the partial FOTA, so no need to build another base Flame build specifically for this issue.

There's a plan to pull NFC out of the gonk layer, and that will require a new FOTA when that work is done.
Flags: needinfo?(nhirata.bugzilla)
Here are the variables for the build I ended up on after going through the FOTA process.

Environmental Variables:
Device: Flame 2.6
BuildID: 20160120030243
Gaia: 43852628a9d506c65525cceb5789b257cc939fe8
Gecko: 2e50b83954e62d52d2ef294e850c4380d457d96a
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 46.0a1 (2.6) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:46.0) Gecko/46.0 Firefox/46.0
QA Whiteboard: [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.