Closed Bug 1275095 Opened 3 years ago Closed 3 years ago

test_get_last_visited.html is going to permafail when Gecko 49 merges to Beta

Categories

(Core :: DOM: Push Notifications, defect, major)

Unspecified
Android
defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox47 --- unaffected
firefox48 + fixed
firefox49 + fixed

People

(Reporter: RyanVM, Assigned: jchen)

References

Details

Attachments

(1 file)

https://treeherder.mozilla.org/logviewer.html#?job_id=20978022&repo=try#L3831

13:38:29     INFO -  247 INFO TEST-START | mobile/android/tests/browser/chrome/test_get_last_visited.html
13:38:52     INFO -  248 INFO SpawnTask.js | Entering test test_get_last_visited
13:38:52     INFO -  249 INFO TEST-UNEXPECTED-FAIL | mobile/android/tests/browser/chrome/test_get_last_visited.html | No listeners for request
13:38:52     INFO -  add_task/</<@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:282:15
13:38:52     INFO -  onRejected@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:85:15
13:38:52     INFO -  250 INFO TEST-OK | mobile/android/tests/browser/chrome/test_get_last_visited.html | took 18193ms
Flags: needinfo?(kcambridge)
jchen, is this related to bug 1267419?
Flags: needinfo?(kcambridge) → needinfo?(nchen)
I think it's because MOZ_ANDROID_GCM is only enabled for nightly builds.
Flags: needinfo?(nchen)
Andrew, I'm being told that the DOM team should own this. Can you please help find an assignee?
Flags: needinfo?(overholt)
William, can you investigate?
Flags: needinfo?(overholt) → needinfo?(wchen)
Tracking for 49, we should fix this within the next aurora cycle.
(In reply to Jim Chen [:jchen] [:darchons] from comment #2)
> I think it's because MOZ_ANDROID_GCM is only enabled for nightly builds.

You're right. The test sends a message [1] that is only handled when MOZ_ANDROID_CGM is enabled [2] because we don't register the message event listeners [3] otherwise.

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/chrome/test_get_last_visited.html#29
[2] https://dxr.mozilla.org/mozilla-central/rev/4d63dde701b47b8661ab7990f197b6b60e543839/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java#192
[3] https://dxr.mozilla.org/mozilla-central/rev/4d63dde701b47b8661ab7990f197b6b60e543839/mobile/android/base/java/org/mozilla/gecko/push/PushService.java#222

The "History:GetPrePathLastVisitedTimeMilliseconds" event doesn't actually depend on the PushService (although it's the only consumer) so we could move it somewhere else.

However, I think it's better for now to modify the test to only run when MOZ_ANDROID_CGM is enabled since the test is testing "History:GetPrePathLastVisitedTimeMilliseconds" and in the current code that only makes sense when MOZ_ANDROID_CGM is enabled.
Currently, MOZ_ANDROID_CGM is only enabled for Nightly builds. We want
to enable it for all builds and let it ride the trains through 48.
Attachment #8758813 - Flags: review?(snorp)
Assignee: nobody → nchen
Flags: needinfo?(wchen)
Comment on attachment 8758813 [details] [diff] [review]
Enable MOZ_ANDROID_CGM for all builds (v1)

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

Whoops!
Attachment #8758813 - Flags: review?(snorp) → review+
Jim please make sure this gets uplifted to 48 as well.
Flags: needinfo?(nchen)
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/edcb7e44c957
Enable MOZ_ANDROID_CGM for all builds; r=snorp
Comment on attachment 8758813 [details] [diff] [review]
Enable MOZ_ANDROID_CGM for all builds (v1)

Uplifting to 48.

Approval Request Comment
[Feature/regressing bug #]: Push
[User impact if declined]: Push notifications will not work
[Describe test coverage new/current, TreeHerder]: None
[Risks and why]: Very small; patch turns on build flag for Aurora, and that build flag has been on in Nightly for a while
[String/UUID change made/needed]: None
Flags: needinfo?(nchen)
Attachment #8758813 - Flags: approval-mozilla-aurora?
Based on Comment 7, tracking this for 48 and marking the 48 branch as affected.
https://hg.mozilla.org/mozilla-central/rev/edcb7e44c957
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8758813 [details] [diff] [review]
Enable MOZ_ANDROID_CGM for all builds (v1)

Let's uplift this before the merge so that push notifications will work.
Attachment #8758813 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
has problems to uplift like 

Tomcats-MacBook-Pro-2:mozilla-central Tomcat$ hg graft --edit -r edcb7e44c957
grafting 348229:edcb7e44c957 "Bug 1275095 - Enable MOZ_ANDROID_CGM for all builds; r=snorp"
merging mobile/android/moz.configure
warning: conflicts while merging mobile/android/moz.configure! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(nchen)
Blocks: 1278435
snorp if you have a chance, can you have a look at the merge conflict and give us a hand getting this landed on aurora? Thanks. I figure maybe Jim is traveling already for the work week.
(see comment 17)
Flags: needinfo?(snorp)
Rebased and uplifted. Could we please uplift bug 1278851 so that `ServiceWorkerRegistration::showNotification` works, too?
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
Verified
Fennec 48.0b1
Build ID 20160607197837
User Agent Mozilla/5.0 (Android 4.2.2; Tablet; rv:48.0) Gecko/48.0 Firefox/48.0
Status: RESOLVED → VERIFIED
Resolution: FIXED → WORKSFORME
How are you verifying a test-only fix?
Flags: needinfo?(mfunches)
Resolution: WORKSFORME → FIXED
Comment 21 should be deleted, was not paying attention to where I posted. Thanks
Flags: needinfo?(mfunches)
You need to log in before you can comment on or make changes to this bug.