Closed Bug 1475264 Opened 2 years ago Closed 2 years ago

Disable new evaluate.isCyclic check from bug 1447977

Categories

(Testing :: Marionette, enhancement, P1, major)

enhancement

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: ato, Assigned: ato)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

https://bugzilla.mozilla.org/show_bug.cgi?id=1447977 introduced a
new implementation of evaluate.isCyclic that was not based on
JSON.stringify to overcome cyclic references stored on Element
prototypes.

We suspect this may have caused an intermittency spike recorded in
https://bugzilla.mozilla.org/show_bug.cgi?id=1414495#c211 (although
probably unrelated to the actual bug itself).

This bug provides a patch which reverts this behaviour without
backing out the original changeset.
Assignee: nobody → ato
Severity: normal → major
Status: NEW → ASSIGNED
Priority: -- → P1
Bug 1447977 introduced a new implementation of evaluate.isCyclic
that was not based on JSON.stringify to overcome cyclic references
stored on Element prototypes.

This patch reverts the new behaviour and hides it behind an environment
variable MOZ_MARIONETTE_NEW_CYCLIC.  This is useful so we can more
easily test the difference in automation.

MozReview-Commit-ID: 2xtNL596Imc
Blocks: 1414495
Bug 1447977 introduced a new implementation of evaluate.isCyclic
that was not based on JSON.stringify to overcome cyclic references
stored on Element prototypes.

This patch reverts the new behaviour and hides it behind an environment
variable MOZ_MARIONETTE_NEW_CYCLIC.  This is useful so we can more
easily test the difference in automation.

MozReview-Commit-ID: 2xtNL596Imc
Attachment #8991641 - Attachment is obsolete: true
Did you make any further discoveries regarding
https://bugzilla.mozilla.org/show_bug.cgi?id=1414495#c211?

I wonder if we should land this patch temporarily to see if it
reduces the OF over the next week?
Flags: needinfo?(aryx.bugmail)
Comment on attachment 8991646 [details] [diff] [review]
Temporarily disable new cylical check in Marionette. r=me, a=Aryx

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

(In reply to Andreas Tolfsen 「:ato」 from comment #4)
> Did you make any further discoveries regarding
> https://bugzilla.mozilla.org/show_bug.cgi?id=1414495#c211?
No.

> I wonder if we should land this patch temporarily to see if it
> reduces the OF over the next week?
Yes, please give it a try.
Attachment #8991646 - Flags: review+
Flags: needinfo?(aryx.bugmail)
Thanks.

Next merge date is in ~two months so this seems like a good time
to experiment.
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7334a57640f
Temporarily disable new cylical check in Marionette. r=me, a=Aryx
https://hg.mozilla.org/mozilla-central/rev/b7334a57640f
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Duplicate of this bug: 1478436
Backout by cbrindusan@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/12f42d129762
Backed out changeset b7334a57640f at ato's request for causing spiking intermittents. a=backout
Target Milestone: mozilla63 → ---
Backed out changeset b7334a57640f (bug 1475264) at ato's request for causing spiking intermittents.

Backout: https://hg.mozilla.org/mozilla-central/rev/12f42d129762967c3f912085787ad489ec97feaa
Flags: needinfo?(ato)
What failed?  Can you provide a link to the Treeherder job?
Flags: needinfo?(cbrindusan)
This backout was made at your request (on IRC/developers), so I'm unable to provide more details.

"ato> CosminS|sheriffduty, CristianB|sheriffduty: Can either of you back out https://bugzilla.mozilla.org/show_bug.cgi?id=1475264?"
Flags: needinfo?(cbrindusan)
Oh sorry, I was confused by the fact that I thought it was a backout
of the backout.  I didn’t expect a needinfo on this.  All’s good!
Flags: needinfo?(ato)
So we can close as wontfix now?
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.