Update WR (render notifier API changes). #18986

glennw · 2017-10-23T01:24:40Z

These changes fix #13480.

This change is

highfive · 2017-10-23T01:24:45Z

Heads up! This PR modifies the following files:

@paulrouget: components/compositing/compositor_thread.rs, components/servo/lib.rs, components/compositing/compositor.rs, components/compositing/lib.rs

glennw · 2017-10-23T01:25:30Z

This includes your commit in #18976, and a follow up commit that (a) updates Cargo.lock (b) updates your commit with the final WR changes.

Your original commit looks good to me.

emilio · 2017-10-23T09:07:07Z

components/compositing/compositor.rs

        RenderNotifier {
            compositor_proxy: compositor_proxy,
        }
    }
 }

 impl webrender_api::RenderNotifier for RenderNotifier {
-    fn new_frame_ready(&mut self) {
+    fn clone(&self) -> Box<webrender_api::RenderNotifier> {
+        Box::new(RenderNotifier::new(self.compositor_proxy.clone()))


Can this be just Box::new(self.clone())

I feel like it becomes awkward calling self.clone() inside of a method named clone. I would rather leave it.

jdm · 2017-10-23T17:39:04Z

@bors-servo: r+

bors-servo · 2017-10-23T17:39:05Z

📌 Commit be9d521 has been approved by jdm

bors-servo · 2017-10-23T18:51:49Z

⌛ Testing commit be9d521 with merge 51ba23d...

Update WR (render notifier API changes). These changes fix #13480.  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18986)

bors-servo · 2017-10-23T19:36:28Z

💔 Test failed - linux-rel-css

jdm · 2017-10-23T20:05:43Z

It looks like something changed related to borders since the last WR update:

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-color-applies-to-003.htm
  └   → /css21_dev/html4/border-left-color-applies-to-003.htm 013a961068b0ccf85b71b6a479cc98e9fc77e102
/css21_dev/html4/reference/border-left-applies-to-001-ref.htm cef4f31905a1f467d5997afb9f0898584bf7d55f
Testing 013a961068b0ccf85b71b6a479cc98e9fc77e102 == cef4f31905a1f467d5997afb9f0898584bf7d55f

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-applies-to-002.htm
  └   → /css21_dev/html4/border-left-applies-to-002.htm 013a961068b0ccf85b71b6a479cc98e9fc77e102
/css21_dev/html4/reference/border-left-applies-to-001-ref.htm cef4f31905a1f467d5997afb9f0898584bf7d55f
Testing 013a961068b0ccf85b71b6a479cc98e9fc77e102 == cef4f31905a1f467d5997afb9f0898584bf7d55f

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-width-applies-to-003.htm
  └   → /css21_dev/html4/border-left-width-applies-to-003.htm 293911574a59c7f3947436264557174a97acc7a5
/css21_dev/html4/reference/ref-filled-black-96px-square.htm f73d1fa4091b541ed29c5bc4f99358740860d9e8
Testing 293911574a59c7f3947436264557174a97acc7a5 == f73d1fa4091b541ed29c5bc4f99358740860d9e8

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-color-applies-to-001.htm
  └   → /css21_dev/html4/border-left-color-applies-to-001.htm 013a961068b0ccf85b71b6a479cc98e9fc77e102
/css21_dev/html4/reference/border-left-applies-to-001-ref.htm cef4f31905a1f467d5997afb9f0898584bf7d55f
Testing 013a961068b0ccf85b71b6a479cc98e9fc77e102 == cef4f31905a1f467d5997afb9f0898584bf7d55f

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-color-applies-to-002.htm
  └   → /css21_dev/html4/border-left-color-applies-to-002.htm 013a961068b0ccf85b71b6a479cc98e9fc77e102
/css21_dev/html4/reference/border-left-applies-to-001-ref.htm cef4f31905a1f467d5997afb9f0898584bf7d55f
Testing 013a961068b0ccf85b71b6a479cc98e9fc77e102 == cef4f31905a1f467d5997afb9f0898584bf7d55f

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-width-applies-to-002.htm
  └   → /css21_dev/html4/border-left-width-applies-to-002.htm 293911574a59c7f3947436264557174a97acc7a5
/css21_dev/html4/reference/ref-filled-black-96px-square.htm f73d1fa4091b541ed29c5bc4f99358740860d9e8
Testing 293911574a59c7f3947436264557174a97acc7a5 == f73d1fa4091b541ed29c5bc4f99358740860d9e8

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-width-applies-to-001.htm
  └   → /css21_dev/html4/border-left-width-applies-to-001.htm 293911574a59c7f3947436264557174a97acc7a5
/css21_dev/html4/reference/ref-filled-black-96px-square.htm f73d1fa4091b541ed29c5bc4f99358740860d9e8
Testing 293911574a59c7f3947436264557174a97acc7a5 == f73d1fa4091b541ed29c5bc4f99358740860d9e8

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-color-applies-to-001.htm
  └   → /css21_dev/html4/border-top-color-applies-to-001.htm f89297eefbde67ac8aecc56f3c1babac2ff57c50
/css21_dev/html4/reference/border-bottom-applies-to-001-ref.htm a8aa4b5a520585465f75aab08fdbe30bed6eb1f4
Testing f89297eefbde67ac8aecc56f3c1babac2ff57c50 == a8aa4b5a520585465f75aab08fdbe30bed6eb1f4

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-width-applies-to-003.htm
  └   → /css21_dev/html4/border-top-width-applies-to-003.htm 293911574a59c7f3947436264557174a97acc7a5
/css21_dev/html4/reference/ref-filled-black-96px-square.htm f73d1fa4091b541ed29c5bc4f99358740860d9e8
Testing 293911574a59c7f3947436264557174a97acc7a5 == f73d1fa4091b541ed29c5bc4f99358740860d9e8

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-width-applies-to-002.htm
  └   → /css21_dev/html4/border-top-width-applies-to-002.htm 293911574a59c7f3947436264557174a97acc7a5
/css21_dev/html4/reference/ref-filled-black-96px-square.htm f73d1fa4091b541ed29c5bc4f99358740860d9e8
Testing 293911574a59c7f3947436264557174a97acc7a5 == f73d1fa4091b541ed29c5bc4f99358740860d9e8

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-color-applies-to-003.htm
  └   → /css21_dev/html4/border-top-color-applies-to-003.htm f89297eefbde67ac8aecc56f3c1babac2ff57c50
/css21_dev/html4/reference/border-bottom-applies-to-001-ref.htm a8aa4b5a520585465f75aab08fdbe30bed6eb1f4
Testing f89297eefbde67ac8aecc56f3c1babac2ff57c50 == a8aa4b5a520585465f75aab08fdbe30bed6eb1f4

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-applies-to-002.htm
  └   → /css21_dev/html4/border-top-applies-to-002.htm f89297eefbde67ac8aecc56f3c1babac2ff57c50
/css21_dev/html4/reference/border-bottom-applies-to-001-ref.htm a8aa4b5a520585465f75aab08fdbe30bed6eb1f4
Testing f89297eefbde67ac8aecc56f3c1babac2ff57c50 == a8aa4b5a520585465f75aab08fdbe30bed6eb1f4

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-applies-to-001.htm
  └   → /css21_dev/html4/border-left-applies-to-001.htm 013a961068b0ccf85b71b6a479cc98e9fc77e102
/css21_dev/html4/reference/border-left-applies-to-001-ref.htm cef4f31905a1f467d5997afb9f0898584bf7d55f
Testing 013a961068b0ccf85b71b6a479cc98e9fc77e102 == cef4f31905a1f467d5997afb9f0898584bf7d55f

  ▶ FAIL [expected PASS] /css21_dev/html4/border-left-applies-to-003.htm
  └   → /css21_dev/html4/border-left-applies-to-003.htm 013a961068b0ccf85b71b6a479cc98e9fc77e102
/css21_dev/html4/reference/border-left-applies-to-001-ref.htm cef4f31905a1f467d5997afb9f0898584bf7d55f
Testing 013a961068b0ccf85b71b6a479cc98e9fc77e102 == cef4f31905a1f467d5997afb9f0898584bf7d55f

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-color-applies-to-002.htm
  └   → /css21_dev/html4/border-top-color-applies-to-002.htm f89297eefbde67ac8aecc56f3c1babac2ff57c50
/css21_dev/html4/reference/border-bottom-applies-to-001-ref.htm a8aa4b5a520585465f75aab08fdbe30bed6eb1f4
Testing f89297eefbde67ac8aecc56f3c1babac2ff57c50 == a8aa4b5a520585465f75aab08fdbe30bed6eb1f4

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-width-applies-to-001.htm
  └   → /css21_dev/html4/border-top-width-applies-to-001.htm 293911574a59c7f3947436264557174a97acc7a5
/css21_dev/html4/reference/ref-filled-black-96px-square.htm f73d1fa4091b541ed29c5bc4f99358740860d9e8
Testing 293911574a59c7f3947436264557174a97acc7a5 == f73d1fa4091b541ed29c5bc4f99358740860d9e8

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-applies-to-001.htm
  └   → /css21_dev/html4/border-top-applies-to-001.htm f89297eefbde67ac8aecc56f3c1babac2ff57c50
/css21_dev/html4/reference/border-bottom-applies-to-001-ref.htm a8aa4b5a520585465f75aab08fdbe30bed6eb1f4
Testing f89297eefbde67ac8aecc56f3c1babac2ff57c50 == a8aa4b5a520585465f75aab08fdbe30bed6eb1f4

  ▶ FAIL [expected PASS] /css21_dev/html4/border-top-applies-to-003.htm
  └   → /css21_dev/html4/border-top-applies-to-003.htm f89297eefbde67ac8aecc56f3c1babac2ff57c50
/css21_dev/html4/reference/border-bottom-applies-to-001-ref.htm a8aa4b5a520585465f75aab08fdbe30bed6eb1f4
Testing f89297eefbde67ac8aecc56f3c1babac2ff57c50 == a8aa4b5a520585465f75aab08fdbe30bed6eb1f4

glennw · 2017-10-23T20:07:46Z

Interesting ... Gecko seems to have updated without any test failures. I'll investigate today.

jdm · 2017-10-23T20:12:37Z

My money is on servo/webrender@635d144 since all of the failing tests have no borders drawn according to the reftest analyzer.

jdm · 2017-10-23T20:15:27Z

It may just be that we were drawing the desired borders in those tests for incorrect reasons before, and the new test results are a more accurate reflection of our table border inadequacies.

glennw · 2017-10-23T23:24:50Z

@bors-servo try

bors-servo · 2017-10-23T23:24:54Z

⌛ Trying commit 1da9dc9 with merge 5f886a2...

Update WR (render notifier API changes). These changes fix #13480.  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18986)

bors-servo · 2017-10-24T01:38:03Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True

glennw · 2017-10-24T01:52:25Z

Disabled those tests due to #18999.

r? @jdm

jdm · 2017-10-24T02:04:51Z

@bors-servo: r+

bors-servo · 2017-10-24T02:04:52Z

📌 Commit 1da9dc9 has been approved by jdm

bors-servo · 2017-10-24T02:04:56Z

⌛ Testing commit 1da9dc9 with merge 2bcb3b4...

Update WR (render notifier API changes). These changes fix #13480.  This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18986)

bors-servo · 2017-10-24T03:38:30Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing 2bcb3b4 to master...

Update webrender for latest RenderNotifier changes.

99b5eac

highfive assigned asajeffrey Oct 23, 2017

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 23, 2017

highfive assigned jdm and unassigned asajeffrey Oct 23, 2017

glennw mentioned this pull request Oct 23, 2017

Update webrender for latest RenderNotifier changes #18976

Closed

4 tasks

emilio approved these changes Oct 23, 2017

View reviewed changes

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 23, 2017

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 23, 2017

Update WR (render notifier API changes).

1da9dc9

glennw force-pushed the update-wr-notifier branch from be9d521 to 1da9dc9 Compare October 23, 2017 23:22

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 23, 2017

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 24, 2017

bors-servo merged commit 1da9dc9 into servo:master Oct 24, 2017

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update WR (render notifier API changes). #18986

Update WR (render notifier API changes). #18986

glennw commented Oct 23, 2017 •

edited by jdm

highfive commented Oct 23, 2017

glennw commented Oct 23, 2017

emilio Oct 23, 2017

jdm Oct 23, 2017

jdm commented Oct 23, 2017

bors-servo commented Oct 23, 2017

bors-servo commented Oct 23, 2017

bors-servo commented Oct 23, 2017

jdm commented Oct 23, 2017

glennw commented Oct 23, 2017

jdm commented Oct 23, 2017

jdm commented Oct 23, 2017

glennw commented Oct 23, 2017

bors-servo commented Oct 23, 2017

bors-servo commented Oct 24, 2017

glennw commented Oct 24, 2017

jdm commented Oct 24, 2017

bors-servo commented Oct 24, 2017

bors-servo commented Oct 24, 2017

bors-servo commented Oct 24, 2017

Update WR (render notifier API changes). #18986

Update WR (render notifier API changes). #18986

Conversation

glennw commented Oct 23, 2017 • edited by jdm

highfive commented Oct 23, 2017

glennw commented Oct 23, 2017

emilio Oct 23, 2017

Choose a reason for hiding this comment

jdm Oct 23, 2017

Choose a reason for hiding this comment

jdm commented Oct 23, 2017

bors-servo commented Oct 23, 2017

bors-servo commented Oct 23, 2017

bors-servo commented Oct 23, 2017

jdm commented Oct 23, 2017

glennw commented Oct 23, 2017

jdm commented Oct 23, 2017

jdm commented Oct 23, 2017

glennw commented Oct 23, 2017

bors-servo commented Oct 23, 2017

bors-servo commented Oct 24, 2017

glennw commented Oct 24, 2017

jdm commented Oct 24, 2017

bors-servo commented Oct 24, 2017

bors-servo commented Oct 24, 2017

bors-servo commented Oct 24, 2017

glennw commented Oct 23, 2017 •

edited by jdm