Closed Bug 1313825 Opened 3 years ago Closed 3 years ago

Investigate and enable the browser_UsageTelemetry_content_aboutHome.js test

Categories

(Firefox :: Search, defect, P1)

defect
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: Dexter, Assigned: Dexter)

References

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

Points: --- → 2
Priority: -- → P1
Whiteboard: [measurement:client]
Assignee: nobody → alessio.placitelli
fwiw, the AboutHomeLoadSnippetsCompleted is fired by the content process, so it would have been better to use ContentTaskUtils.waitForEvent. That said, that is unlikely to fix the problem.

At this point you likely need to add a bunch of debugging to the test, like checking that the ContentSearchController is alive, where we are in the about:home init and so on, to try to get a dump when it fails.
Blocks: 1303333
Attached patch bug1313825.patchSplinter Review
(In reply to Marco Bonardo [::mak] from comment #1)
> fwiw, the AboutHomeLoadSnippetsCompleted is fired by the content process, so
> it would have been better to use ContentTaskUtils.waitForEvent. That said,
> that is unlikely to fix the problem.

This patch seems to fix the problem, see [1] (the failures don't relate to this test).
I tried to use |ContentTaskUtils.waitForEvent| but it gets stuck and never moves on. "AboutHomeLoadSnippetsCompleted" seems to be a chrome event: is this the reason?

[1] - https://treeherder.mozilla.org/#/jobs?repo=try&revision=847370d6a38a&selectedJob=30228334
Attachment #8806336 - Flags: review?(mak77)
(In reply to Alessio Placitelli [:Dexter] from comment #2)
> I tried to use |ContentTaskUtils.waitForEvent| but it gets stuck and never
> moves on. "AboutHomeLoadSnippetsCompleted" seems to be a chrome event: is
> this the reason?

I think it's more likely that the event fires before the listener is added. the event is created in content and bubbles up, indeed the addEventListener has the wantsUntrusted boolean set.
Btw, let's not fight windmills, a working solution is enough :)
Comment on attachment 8806336 [details] [diff] [review]
bug1313825.patch

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

Let's see how this behaves.
Attachment #8806336 - Flags: review?(mak77) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffd43dc2055f87b29c49390559d8edade1ff6479
Bug 1313825 - Fix and enable browser_UsageTelemetry_content_aboutHome.js. r=mak
https://hg.mozilla.org/mozilla-central/rev/ffd43dc2055f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Comment on attachment 8806336 [details] [diff] [review]
bug1313825.patch

Approval Request Comment
[Feature/regressing bug #]: This patch fixes and enables a test from bug 1303333. It needs to be uplifted together with that bug.
[User impact if declined]: None.
[Describe test coverage new/current, TreeHerder]: It enables a new test guards against data regressions due to changes in the FF code.
[Risks and why]: None.
[String/UUID change made/needed]: None
Attachment #8806336 - Flags: approval-mozilla-aurora?
Comment on attachment 8806336 [details] [diff] [review]
bug1313825.patch

This patch fixes a test error. Take it in 51 aurora.
Attachment #8806336 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.