Closed Bug 1148949 Opened 8 years ago Closed 8 years ago

Intermittent test_deferred_start.html | Starting an animation with a delay starts from the correct point - Starting an animation with a delay starts from the correct point: assert_equals: Got a valid transform matrix on the compositor (got: "") expected 6

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox38 --- unaffected
firefox39 --- fixed
firefox40 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: philor, Assigned: birtles)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Apply async properties when querying the animated transform for cross-process compositor parents too
4.21 KB, patch
mattwoodrow
: review+
lmandel
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
      No description provided.
Summary: Intermittent Mulet dom/animation/test/mozilla/test_deferred_start.html | Starting an animation with a delay starts from the correct point - Starting an animation with a delay starts from the correct point: assert_equals: Got a valid transform matrix on th → Intermittent test_deferred_start.html | Starting an animation with a delay starts from the correct point - Starting an animation with a delay starts from the correct point: assert_equals: Got a valid transform matrix on the compositor (got: "") expected 6
I think I know what's going on here. Patch coming up.
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Bug 1113425 (specifically, part 2:
https://hg.mozilla.org/mozilla-central/rev/bb3866dea03e) introduced a fix for
a race condition that occurs when querying the animated transform of a layer.
However, that fix failed to update CrossProcessCompositorParent and hence the
issue still arises when e10s is enabled.

This patch applies the fix from that bug to CrossProcessCompositorParent.
Attachment #8595107 - Flags: review?(matt.woodrow)
Blocks: 1150351
Comment on attachment 8595107 [details] [diff] [review]
Apply async properties when querying the animated transform for cross-process compositor parents too

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

::: gfx/layers/ipc/CompositorParent.cpp
@@ +2028,5 @@
>  }
>  
>  void
> +CrossProcessCompositorParent::ApplyAsyncProperties(LayerTransactionParent*
> +                                                     aLayerTree)

Indenting seems wrong here.
Attachment #8595107 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/11d74f18ce0c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Please nominate this for Aurora approval when you get a chance.
status-firefox38: --- → unaffected
status-firefox39: --- → affected
status-firefox-esr31: --- → unaffected
Flags: needinfo?(bbirtles)
Comment on attachment 8595107 [details] [diff] [review]
Apply async properties when querying the animated transform for cross-process compositor parents too

Approval Request Comment
[Feature/regressing bug #]: 980770
[User impact if declined]: test_deferred_start.html mochitest fails intermittently
[Describe test coverage new/current, TreeHerder]: seems to be behaving itself on trunk, try run is pretty convincing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=18b5bc538cf8
[Risks and why]: Minimal. This only affects test code when OMTA is enabled.
[String/UUID change made/needed]: None
Flags: needinfo?(bbirtles)
Attachment #8595107 - Flags: approval-mozilla-aurora?
Comment on attachment 8595107 [details] [diff] [review]
Apply async properties when querying the animated transform for cross-process compositor parents too

Aurora+
Attachment #8595107 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Two failures on m-i in one day when we didn't have any for a week suggests something landed recently on m-i that regressed this.
The best I can see in that window is:

https://hg.mozilla.org/integration/mozilla-inbound/rev/7e9c4abf391b

We'll get another chance to narrow that down when it hits m-c.
These last two failures are incorrectly filed, but are likely fallout from bug 1159743.
This first fails m-c after the following push:

  https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=34828fed1639

However, nothing in that looks suspicious. The previous merge from m-i to m-c is:

  https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=0f5eacc986e8

which includes some tweaks to OMTA such as:

  https://hg.mozilla.org/mozilla-central/rev/962e15b81684 (bug 947753)

That landed 5 hours before the failure in comment 54 so it could be related, but wouldn't explain the two failures in comment 47 and comment 48.

Changeset 962e15b81684 is going to get backed out in bug 1161049 so we'll see if that makes any difference. If not, then it could possibly be the vsync changes in https://hg.mozilla.org/integration/mozilla-inbound/rev/7e9c4abf391b although that seems unlikely.
See Also: → 1168759
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.