1235050 - Simple Push doesn't work anymore

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Jovan Gerodetti]

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.

Comment 1

[:gerard-majax]

Is it possible it is the same as bug 1174420 ?

Comment 2

[:gerard-majax]

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$

Comment 3

[:gerard-majax]

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 ...

Comment 4

[:gerard-majax]

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 ...

Comment 5

[Jovan Gerodetti]

(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.

Comment 6

[:gerard-majax]

Attachment: Fix SimplePush packaging on B2G r=...

Fixes the issue for me.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6ce27a7a677

Comment 7

[:gerard-majax]

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.

Comment 8

[:gerard-majax]

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

Comment 9

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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.

Comment 10

[:gerard-majax]

fix and tests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=612953c7b3e0
tests only: https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf8086251e83

Comment 11

[:gerard-majax]

Attachment: Add basic SimplePush testing r=...

Comment 12

[:gerard-majax]

Attachment: Fix SimplePush packaging on B2G r=...

Comment 13

[:gerard-majax]

and making sure nothing breaks: https://treeherder.mozilla.org/#/jobs?repo=try&revision=96a4c7794e34

Comment 14

[:gerard-majax]

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

Comment 15

[:gerard-majax]

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

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

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.

Comment 17

[:gerard-majax]

(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 :)

Comment 18

[:gerard-majax]

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

Comment 19

[:gerard-majax]

(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().

Comment 20

[:gerard-majax]

fix inside: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c396c0f14d1
tests only: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5db3e12f4b6

Comment 21

[Lina Butler [:lina]]

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?

Comment 22

[:gerard-majax]

(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 !

Comment 23

[:gerard-majax]

Attachment: Fix SimplePush packaging on B2G

Comment 24

[:gerard-majax]

Attachment: Add basic SimplePush testing

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/1db8e4432568
https://hg.mozilla.org/integration/b2g-inbound/rev/29aa734ee232

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1db8e4432568
https://hg.mozilla.org/mozilla-central/rev/29aa734ee232

Comment 27

[Jovan Gerodetti]

Will this be in tomorrows build?

Comment 28

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Yes, there should be a 3 pm PDT build that contains this fix for flame-latest.  It should also be in tomorrow's build.

Comment 29

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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.

Comment 30

[Johan Lorenzo [:jlorenzo]]

(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.

Comment 31

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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.

Comment 32

[Adam Alldredge [:AdamA][Inactive]]

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