Skip to content

/ servo

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

Store the pointer of reusable sheets in StylesheetLoader. #17123

Merged
merged 2 commits into from Jun 5, 2017
Merged

Store the pointer of reusable sheets in StylesheetLoader. #17123

merged 2 commits into from Jun 5, 2017

Conversation

@kuoe0
Copy link
Contributor

kuoe0 commented Jun 1, 2017 •
edited by larsbergstrom

To make stylo support reusable style sheets, we need to store the pointer of reusable style sheets we got from gecko. And we pass them back to gecko to reuse.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix Bug 1358993.
  • There are tests for these changes OR
  • These changes do not require tests because the test cases are in Gecko.

This change is Reviewable
@highfive
Copy link

highfive commented Jun 1, 2017

Heads up! This PR modifies the following files:

  • @emilio: ports/geckolib/stylesheet_loader.rs, ports/geckolib/glue.rs
@kuoe0 kuoe0 mentioned this pull request Jun 1, 2017
Open
@kuoe0 kuoe0 force-pushed the kuoe0:bug-1358993 branch from da310dc to f8fa3f2 Jun 1, 2017
@emilio
Copy link
Member

emilio commented Jun 1, 2017

You need to update the checked-in bindings for CI to pass.
@kuoe0 kuoe0 force-pushed the kuoe0:bug-1358993 branch 2 times, most recently from d6cde69 to 83d5318 Jun 2, 2017
@kuoe0
Copy link
Contributor Author

kuoe0 commented Jun 2, 2017

@emilio I meet this problem. Is anything wrong in my patch?

test bindings::root::mozilla::css::bindgen_test_layout_Loader_Sheets ... FAILED

failures:

---- bindings::root::mozilla::css::bindgen_test_layout_Loader_Sheets stdout ----

	thread 'bindings::root::mozilla::css::bindgen_test_layout_Loader_Sheets' panicked at 
        'assertion failed: `(left == right)` (left: `104`, right: `144`): Size of: Loader_Sheets', 
        /home/travis/build/servo/servo/target/geckolib/debug/build/style-f29221ef6c8d579d/out/gecko/structs_debug.rs:1768
@bors-servo
Copy link
Contributor

bors-servo commented Jun 2, 2017

☔ The latest upstream changes (presumably #17137) made this pull request unmergeable. Please resolve the merge conflicts.
@nox
Copy link
Member

nox commented Jun 2, 2017

These 2 commits need to be squashed together, or at least the second one should come first. Also, this needs a rebase.
@nox nox removed the S-awaiting-review label Jun 2, 2017
@emilio
Copy link
Member

emilio commented Jun 2, 2017

So that looks like a bindgen issue because of the loader sheets getting pulled in, and bindgen not being able to deduce the size of:

    nsBaseHashtable<URIPrincipalReferrerPolicyAndCORSModeHashKey,
                    RefPtr<StyleSheet>,
                    StyleSheet*> mCompleteSheets;

I'd recomend hiding that type for now, since we don't use it. You can do it adding nsBaseHashtable (or css::Loader::Sheets) to the opaque-types list in layout/style/ServoBindings.toml.
@kuoe0 kuoe0 force-pushed the kuoe0:bug-1358993 branch from 83d5318 to 2cad2dd Jun 5, 2017
@kuoe0 kuoe0 force-pushed the kuoe0:bug-1358993 branch from 2cad2dd to d8a35ba Jun 5, 2017
kuoe0 added 2 commits May 18, 2017
@kuoe0
Store the pointer of reusable sheets in StylesheetLoader.
3a15cbd 
MozReview-Commit-ID: DZP2bChjj3k
@kuoe0
Update generated files
Loading status checks…
00d3fc2 
MozReview-Commit-ID: 3bQbda2PJLX
@kuoe0 kuoe0 force-pushed the kuoe0:bug-1358993 branch from d8a35ba to 00d3fc2 Jun 5, 2017
@BorisChiou
Copy link
Contributor

BorisChiou commented Jun 5, 2017

@bors-servo delegate+
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2017

✌️ @kuoe0 can now approve this pull request
@kuoe0
Copy link
Contributor Author

kuoe0 commented Jun 5, 2017

r=heycam
@shinglyu
Copy link
Member

shinglyu commented Jun 5, 2017

@kuoe0 You need to tag @bors-servo
@kuoe0
Copy link
Contributor Author

kuoe0 commented Jun 5, 2017

@bors-servo r=heycam
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2017

📌 Commit 00d3fc2 has been approved by heycam
@highfive highfive assigned heycam and unassigned wafflespeanut Jun 5, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2017

⌛ Testing commit 00d3fc2 with merge 63be9d7...
bors-servo added a commit that referenced this pull request Jun 5, 2017
@bors-servo
Auto merge of #17123 - KuoE0:bug-1358993, r=heycam
Loading status checks…
63be9d7 
Store the pointer of reusable sheets in StylesheetLoader.

<!-- Please describe your changes on the following line: -->

To make stylo support reusable style sheets, we need to store the pointer of reusable style sheets we got from gecko. And we pass them back to gecko to reuse.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1358993](https://bugzilla.mozilla.org/show_bug.cgi?id=1358993).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because the test cases are in Gecko.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/17123)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 5, 2017

☀️ 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: heycam
Pushing 63be9d7 to master...
@bors-servo bors-servo merged commit 00d3fc2 into servo:master Jun 5, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@heycam heycam

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
@kuoe0 @highfive @emilio @bors-servo @nox @BorisChiou @shinglyu @heycam @wafflespeanut
You can’t perform that action at this time.