Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ParsedDeclaration::expand with non-generic method #16183

Merged
merged 1 commit into from Mar 29, 2017
Merged

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Mar 29, 2017

... to reduce its code size impact.

https://bugzilla.mozilla.org/show_bug.cgi?id=1351737


This change is Reviewable

@highfive
Copy link

highfive commented Mar 29, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/keyframes.rs, components/style/properties/declaration_block.rs, components/style/properties/properties.mako.rs
  • @KiChjang: components/script/dom/cssstyledeclaration.rs
  • @fitzgen: components/script/dom/cssstyledeclaration.rs
  • @emilio: ports/geckolib/glue.rs, components/style/keyframes.rs, components/style/properties/declaration_block.rs, components/style/properties/properties.mako.rs
@highfive
Copy link

highfive commented Mar 29, 2017

warning Warning warning

  • These commits modify style and script code, but no tests are modified. Please consider adding a test!
@SimonSapin
Copy link
Member Author

SimonSapin commented Mar 29, 2017

@emilio
Copy link
Member

emilio commented Mar 29, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

📌 Commit af037ae has been approved by emilio

@emilio
Copy link
Member

emilio commented Mar 29, 2017

Copy link
Member

emilio left a comment

r=me with those fixed

importance,
overwrite_more_important,
)
||

This comment has been minimized.

@emilio

emilio Mar 29, 2017

Member

This can't short-circuit.

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2017

Member

Do we short circuit things with side effects?

This comment has been minimized.

@Manishearth

Manishearth Mar 29, 2017

Member

Huh, we do. Interesting.

This comment has been minimized.

@SimonSapin

SimonSapin Mar 29, 2017

Author Member

The || operator doesn’t know whether its operands have side effects.

importance,
overwrite_more_important,
)
||

This comment has been minimized.

@emilio

emilio Mar 29, 2017

Member

Nor this.

@Manishearth
Copy link
Member

Manishearth commented Mar 29, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

📌 Commit af037ae has been approved by Manishearth

@highfive highfive assigned Manishearth and unassigned emilio Mar 29, 2017
@Manishearth
Copy link
Member

Manishearth commented Mar 29, 2017

er.

@Manishearth
Copy link
Member

Manishearth commented Mar 29, 2017

@emilio
Copy link
Member

emilio commented Mar 29, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

📌 Commit 4f2b9a3 has been approved by emilio

@highfive highfive assigned emilio and unassigned Manishearth Mar 29, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

Testing commit 4f2b9a3 with merge f3a5ad2...

bors-servo added a commit that referenced this pull request Mar 29, 2017
Replace ParsedDeclaration::expand with non-generic method

... to reduce its code size impact.

https://bugzilla.mozilla.org/show_bug.cgi?id=1351737

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16183)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

💔 Test failed - linux-rel-wpt

@SimonSapin
Copy link
Member Author

SimonSapin commented Mar 29, 2017

  ▶ CRASH [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-1.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  │ 3.3 (Core Profile) Mesa 12.0.1
  │ ERROR:script::dom::bindings::error: Error at http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_helper-1.html:3:1 opener is not defined
  │ called `Result::unwrap()` on an `Err` value: Error { repr: Custom(Custom { kind: UnexpectedEof, error: StringError("failed to fill whole buffer") }) } (thread RenderBackend, at /checkout/src/libcore/result.rs:859)
  │ stack backtrace:
  │    0:     0x7f7f04f0f5fd - backtrace::backtrace::trace::h43650c97fbd1994d
  │    1:     0x7f7f04f0fc02 - backtrace::capture::Backtrace::new::hb5a725a088a2a2fc
  │    2:     0x7f7f047f33d7 - servo::main::{{closure}}::h9d96b5c32a8afffa
  │    3:     0x7f7f0675a13d - std::panicking::rust_panic_with_hook
  │                         at /checkout/src/libstd/panicking.rs:550
  │    4:     0x7f7f06759fd4 - std::panicking::begin_panic<collections::string::String>
  │                         at /checkout/src/libstd/panicking.rs:511
  │    5:     0x7f7f06759f09 - std::panicking::begin_panic_fmt
  │                         at /checkout/src/libstd/panicking.rs:495
  │    6:     0x7f7f06759e97 - std::panicking::rust_begin_panic
  │                         at /checkout/src/libstd/panicking.rs:471
  │    7:     0x7f7f0678669d - core::panicking::panic_fmt
  │                         at /checkout/src/libcore/panicking.rs:69
  │    8:     0x7f7f04c21d39 - core::result::unwrap_failed::h6bfb389c41db9e3b
  │    9:     0x7f7f04c8ee5d - webrender::render_backend::RenderBackend::run::hfa58adaca88f10b0
  │   10:     0x7f7f04c01cff - std::panicking::try::do_call::h17ab1154c25f8791
  │   11:     0x7f7f0676100a - panic_unwind::__rust_maybe_catch_panic
  │                         at /checkout/src/libpanic_unwind/lib.rs:98
  │   12:     0x7f7f04c22ef4 - <F as alloc::boxed::FnBox<A>>::call_box::h87f7ba67e73da805
  │   13:     0x7f7f06758c44 - alloc::boxed::{{impl}}::call_once<(),()>
  │                         at /checkout/src/liballoc/boxed.rs:650
  │                          - std::sys_common::thread::start_thread
  │                         at /checkout/src/libstd/sys_common/thread.rs:21
  │                          - std::sys::imp::thread::{{impl}}::new::thread_start
  │                         at /checkout/src/libstd/sys/unix/thread.rs:84
  │   14:     0x7f7f026be183 - start_thread
  │   15:     0x7f7f021d5bec - clone
  │   16:                0x0 - <unknown>
  │ ERROR:servo: called `Result::unwrap()` on an `Err` value: Error { repr: Custom(Custom { kind: UnexpectedEof, error: StringError("failed to fill whole buffer") }) }
  └ Pipeline failed in hard-fail mode.  Crashing!

@bors-servo retry #14323

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, windows-msvc-dev are reusable. Rebuilding only linux-rel-css, linux-rel-wpt, mac-rel-wpt1, mac-rel-wpt2...

@bors-servo
Copy link
Contributor

bors-servo commented Mar 29, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: emilio
Pushing f3a5ad2 to master...

@bors-servo bors-servo merged commit 4f2b9a3 into master Mar 29, 2017
3 of 4 checks passed
3 of 4 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details
@SimonSapin SimonSapin deleted the expand-diet branch Apr 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.