Move most animation processing to script #26464

mrobinson · 2020-05-07T17:04:26Z

This is preparation for sharing this code with layout_2020 and
implementing selective off-the-main-thread animations.

We still look for nodes not in the flow tree in the layout thread.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because they should not change behavior.

highfive · 2020-05-07T17:04:31Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/document.rs, components/script/lib.rs, components/script/Cargo.toml, components/constellation/constellation.rs, components/script/animations.rs and 4 more
@cbrewster: components/constellation/constellation.rs
@KiChjang: components/script_traits/script_msg.rs, components/script/dom/document.rs, components/script/lib.rs, components/script/Cargo.toml, components/script/animations.rs and 5 more
@emilio: components/layout/lib.rs, components/style/animation.rs, components/layout/animation.rs, components/layout/context.rs

highfive · 2020-05-07T17:04:33Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify style, layout, and script code, but no tests are modified. Please consider adding a test!

mrobinson · 2020-05-07T17:04:36Z

@bors-servo try=wpt

bors-servo · 2020-05-07T17:04:41Z

⌛ Trying commit 69bf5d6 with merge 1bfeb85...


        Auto merge of #26464 - mrobinson:move-animations-to-script, r=<try>

Move most animation processing to script This is preparation for sharing this code with layout_2020 and implementing selective off-the-main-thread animations. We still look for nodes not in the flow tree in the layout thread.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes do not require tests because they should not change behavior.

bors-servo · 2020-05-07T17:49:33Z

☀️ Test successful - status-taskcluster
State: approved= try=True

mrobinson · 2020-05-07T17:57:23Z

@jdm r?

jdm

Great work!

components/layout_thread/lib.rs

jdm · 2020-05-11T17:00:06Z

components/script/animations.rs

+    }
+}
+
+impl MallocSizeOf for Animations {


Can we use #[derive(MallocSizeOf)] on Animations instead?

I'm not sure this works because MallocSizeOf doesn't know what to do with ServoArc. I also cannot implement MallocSizeOf for ServoArc because it's defined outside the crate.

168 | impl MallocSizeOf for Arc<RwLock<FxHashMap<OpaqueNode, ElementAnimationSet>>> { | ^^^^^^^^^^^^^^^^^^^^^^------------------------------------------------------- | | | | | `style::servo_arc::Arc` is not defined in the current crate | impl doesn't use only types from inside the current crate

I have modified this to include the other member of the Animations though. Let me know if there's a better way to accomplish this.

Good point; Arcs always cause us problems. Let's add this comment above Animations:
// Make sure to update the MallocSizeOf implementation when changing the contents of this struct.

components/script/dom/bindings/trace.rs

components/script/dom/window.rs

components/script/script_thread.rs

jdm · 2020-05-11T18:07:22Z

One last comment fix and this can merge! Thanks!

jdm · 2020-05-11T18:09:53Z

@bors-servo r+

bors-servo · 2020-05-11T18:09:54Z

📌 Commit 9fc161f has been approved by jdm

bors-servo · 2020-05-11T18:22:33Z

⌛ Testing commit 9fc161f with merge 13a9479...


        Auto merge of #26464 - mrobinson:move-animations-to-script, r=jdm

Move most animation processing to script This is preparation for sharing this code with layout_2020 and implementing selective off-the-main-thread animations. We still look for nodes not in the flow tree in the layout thread.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes do not require tests because they should not change behavior.

bors-servo · 2020-05-11T19:02:06Z

💔 Test failed - status-taskcluster

mrobinson · 2020-05-11T19:04:36Z

The failure looks like a real issue. I'll take a look tomorrow.

1 unexpected results that are NOT known-intermittents:
  ▶ TIMEOUT [expected OK] /dom/events/webkit-transition-end-event.html
  │ 
  │ error: XDG_RUNTIME_DIR not set in the environment.
  └ libEGL warning: No hardware driver found, falling back to software rendering
  ▶ Unexpected subtest result in /dom/events/webkit-transition-end-event.html:
  │ TIMEOUT [expected PASS] webkitTransitionEnd event listener should not trigger if an unprefixed listener also exists
  └   → Test timed out
  ▶ Unexpected subtest result in /dom/events/webkit-transition-end-event.html:
  └ NOTRUN [expected PASS] webkitTransitionEnd event listener should not trigger if an unprefixed event handler also exists
  ▶ Unexpected subtest result in /dom/events/webkit-transition-end-event.html:
  └ NOTRUN [expected FAIL] event types for prefixed and unprefixed transitionend event listeners should be named appropriately
  ▶ Unexpected subtest result in /dom/events/webkit-transition-end-event.html:
  └ NOTRUN [expected PASS] webkitTransitionEnd event listener is case sensitive


        Move most animation processing to script

This is preparation for sharing this code with layout_2020 and implementing selective off-the-main-thread animations. We still look for nodes not in the flow tree in the layout thread.

mrobinson · 2020-05-12T08:24:27Z

@bors-servo try=wpt

Run wpt tests to see if this is a flake or a consistent failure.

bors-servo · 2020-05-12T08:24:32Z

⌛ Trying commit 3b0619a with merge 930794a...


        Auto merge of #26464 - mrobinson:move-animations-to-script, r=<try>

Move most animation processing to script This is preparation for sharing this code with layout_2020 and implementing selective off-the-main-thread animations. We still look for nodes not in the flow tree in the layout thread.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes do not require tests because they should not change behavior.

bors-servo · 2020-05-12T09:07:49Z

☀️ Test successful - status-taskcluster
State: approved= try=True

mrobinson · 2020-05-12T10:18:44Z

@bors-servo r=jdm

I think this failure might be a rare flake. I'm not able to reproduce it locally at all (despite trying pretty hard). I'm going to try to pass this through bors again and see if there is something about the commit process that is causing it.

bors-servo · 2020-05-12T10:18:45Z

📌 Commit 3b0619a has been approved by jdm

bors-servo · 2020-05-12T10:18:50Z

⌛ Testing commit 3b0619a with merge ebc139d...


        Auto merge of #26464 - mrobinson:move-animations-to-script, r=jdm

Move most animation processing to script This is preparation for sharing this code with layout_2020 and implementing selective off-the-main-thread animations. We still look for nodes not in the flow tree in the layout thread.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes do not require tests because they should not change behavior.

bors-servo · 2020-05-12T11:07:11Z

💔 Test failed - status-taskcluster

mrobinson · 2020-05-12T11:08:47Z

@bors-servo retry

#24622

bors-servo · 2020-05-12T11:08:52Z

⌛ Testing commit 3b0619a with merge c5617ef...

bors-servo · 2020-05-12T11:43:40Z

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing c5617ef to master...

highfive assigned ferjm May 7, 2020

highfive added the S-awaiting-review label May 7, 2020

mrobinson marked this pull request as ready for review May 7, 2020

mrobinson requested a review from jdm May 7, 2020

jdm assigned jdm and unassigned ferjm May 8, 2020

mrobinson mentioned this pull request May 11, 2020

layout_2020: Add animations and transitions support #26486

Merged

4 of 4 tasks complete

jdm requested changes May 11, 2020

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels May 11, 2020

mrobinson force-pushed the mrobinson:move-animations-to-script branch from 69bf5d6 to 938c488 May 11, 2020

highfive added S-awaiting-review and removed S-needs-code-changes labels May 11, 2020

jdm approved these changes May 11, 2020

View changes

mrobinson force-pushed the mrobinson:move-animations-to-script branch from 938c488 to 9fc161f May 11, 2020

highfive added S-awaiting-merge and removed S-awaiting-review labels May 11, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels May 11, 2020

Move most animation processing to script

Loading status checks…

3b0619a

This is preparation for sharing this code with layout_2020 and implementing selective off-the-main-thread animations. We still look for nodes not in the flow tree in the layout thread.

mrobinson force-pushed the mrobinson:move-animations-to-script branch from 9fc161f to 3b0619a May 12, 2020

highfive added S-awaiting-review and removed S-tests-failed labels May 12, 2020

highfive added S-awaiting-merge and removed S-awaiting-review labels May 12, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels May 12, 2020

highfive removed the S-tests-failed label May 12, 2020

highfive added the S-awaiting-merge label May 12, 2020

bors-servo merged commit c5617ef into servo:master May 12, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label May 12, 2020

bors-servo mentioned this pull request May 12, 2020

Implement concept of dirty root #26484

Merged

mrobinson mentioned this pull request May 13, 2020

Intermittent crash in /_mozilla/mozilla/transitionend_safety.html #26498

Closed

servo / servo

Move most animation processing to script #26464

Move most animation processing to script #26464

mrobinson commented May 7, 2020

highfive commented May 7, 2020

highfive commented May 7, 2020

mrobinson commented May 7, 2020

bors-servo commented May 7, 2020

bors-servo commented May 7, 2020

mrobinson commented May 7, 2020

jdm left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jdm commented May 11, 2020

jdm commented May 11, 2020

bors-servo commented May 11, 2020

bors-servo commented May 11, 2020

bors-servo commented May 11, 2020

mrobinson commented May 11, 2020

mrobinson commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

mrobinson commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

mrobinson commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

servo / servo

Join GitHub today

Move most animation processing to script #26464

Move most animation processing to script #26464

Conversation

mrobinson commented May 7, 2020

highfive commented May 7, 2020

highfive commented May 7, 2020

mrobinson commented May 7, 2020

bors-servo commented May 7, 2020

bors-servo commented May 7, 2020

mrobinson commented May 7, 2020

jdm left a comment

This comment has been minimized.

jdm May 11, 2020

This comment has been minimized.

mrobinson May 11, 2020

This comment has been minimized.

jdm May 11, 2020

jdm commented May 11, 2020

jdm commented May 11, 2020

bors-servo commented May 11, 2020

bors-servo commented May 11, 2020

bors-servo commented May 11, 2020

mrobinson commented May 11, 2020

mrobinson commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

mrobinson commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020

mrobinson commented May 12, 2020

bors-servo commented May 12, 2020

bors-servo commented May 12, 2020