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

Don't OOM when laying out basic web pages on Android #20390

Merged
merged 1 commit into from Mar 22, 2018

Conversation

@jdm
Copy link
Member

jdm commented Mar 22, 2018

This is a workaround for the symptom described in #20149 (comment) which looks like either a standard library or compiler bug. My release android build can load all sorts of web pages as expected with this change.


This change is Reviewable

@highfive
Copy link

highfive commented Mar 22, 2018

Heads up! This PR modifies the following files:

  • @emilio: components/layout/inline.rs
@highfive
Copy link

highfive commented Mar 22, 2018

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
@jdm jdm added this to To do in Android MVP via automation Mar 22, 2018
@jdm
Copy link
Member Author

jdm commented Mar 22, 2018

@highfive highfive assigned mbrubeck and unassigned pcwalton Mar 22, 2018
@emilio
emilio approved these changes Mar 22, 2018
Copy link
Member

emilio left a comment

r=me if you want, with an issue filed and referenced from there.

self.last_known_line_breaking_opportunity = None;
mem::replace(&mut self.pending_line,
Line::new(self.floats.writing_mode, &self.minimum_metrics))
self.pending_line = Line::new(self.floats.writing_mode, &self.minimum_metrics);

This comment has been minimized.

@emilio

emilio Mar 22, 2018

Member

Is there a bug on file for rust on this?

This comment has been minimized.

@jdm jdm force-pushed the jdm:android-layout-workaround branch from c969455 to 391d062 Mar 22, 2018
@jdm
Copy link
Member Author

jdm commented Mar 22, 2018

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

bors-servo commented Mar 22, 2018

📌 Commit 391d062 has been approved by emilio

@highfive highfive assigned emilio and unassigned mbrubeck Mar 22, 2018
@jdm jdm moved this from To do to In progress in Android MVP Mar 22, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Mar 22, 2018

Testing commit 391d062 with merge eaf59ca...

bors-servo added a commit that referenced this pull request Mar 22, 2018
Don't OOM when laying out basic web pages on Android

This is a workaround for the symptom described in #20149 (comment) which looks like either a standard library or compiler bug. My release android build can load all sorts of web pages as expected with this change.

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

bors-servo commented Mar 22, 2018

@bors-servo bors-servo merged commit 391d062 into servo:master Mar 22, 2018
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
Android MVP automation moved this from In progress to Done Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Android MVP
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

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