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

Cache XHR JSON responses #9621

Merged
merged 1 commit into from Feb 15, 2016
Merged

Cache XHR JSON responses #9621

merged 1 commit into from Feb 15, 2016

Conversation

@KiChjang
Copy link
Member

KiChjang commented Feb 12, 2016

Fixes #3919.

Review on Reviewable
@highfive
Copy link

highfive commented Feb 12, 2016

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@jdm
Copy link
Member

jdm commented Feb 12, 2016

This isn't a thorough review, just a couple things that jumped out at me:

  • I'm not convinced that having the MutJSHeapValue size be reported as 0 is better than selectively ignoring the field with a useful explanation
  • I'm pretty sure we want is_null() instead of comparing the JS value against a null handle value
@KiChjang
Copy link
Member Author

KiChjang commented Feb 12, 2016

Not sure what the explanation should be though, "managed through JS GC"?
@KiChjang KiChjang force-pushed the KiChjang:xhr-response-json branch 3 times, most recently from 514114a to 3e5220a Feb 12, 2016
@KiChjang KiChjang mentioned this pull request Feb 13, 2016
Closed
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

☔ The latest upstream changes (presumably #9629) made this pull request unmergeable. Please resolve the merge conflicts.
@KiChjang
Cache XHR JSON responses
Loading status checks…
6be5218
@KiChjang KiChjang force-pushed the KiChjang:xhr-response-json branch from 3e5220a to 6be5218 Feb 15, 2016
@jdm jdm removed the S-needs-rebase label Feb 15, 2016
@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 15, 2016

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

bors-servo commented Feb 15, 2016

📌 Commit 6be5218 has been approved by Ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

⌛ Testing commit 6be5218 with merge 94d6ca2...
bors-servo added a commit that referenced this pull request Feb 15, 2016
@bors-servo
Auto merge of #9621 - KiChjang:xhr-response-json, r=Ms2ger
94d6ca2 
Cache XHR JSON responses

Fixes #3919.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9621)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

💔 Test failed - linux-rel
@KiChjang
Copy link
Member Author

KiChjang commented Feb 15, 2016

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

⚡ Previous build results for android, gonk, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only linux-rel...
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

💔 Test failed - linux-rel
@KiChjang
Copy link
Member Author

KiChjang commented Feb 15, 2016

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

⌛ Testing commit 6be5218 with merge 5ee5ce0...
bors-servo added a commit that referenced this pull request Feb 15, 2016
@bors-servo
Auto merge of #9621 - KiChjang:xhr-response-json, r=Ms2ger
Loading status checks…
5ee5ce0 
Cache XHR JSON responses

Fixes #3919.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9621)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

💔 Test failed - linux-rel
@jdm
Copy link
Member

jdm commented Feb 15, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

⚡ Previous build results for android, gonk, linux-dev, mac-dev-unit, mac-rel-css are reusable. Rebuilding only linux-rel, mac-rel-wpt...
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

💔 Test failed - linux-rel
@jdm
Copy link
Member

jdm commented Feb 15, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

⚡ Previous build results for android, gonk, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only linux-rel...
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

💔 Test failed - linux-rel
@KiChjang
Copy link
Member Author

KiChjang commented Feb 15, 2016

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

⚡ Previous build results for android, gonk, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only linux-rel...
@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2016

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo mentioned this pull request Feb 15, 2016
Merged
@bors-servo bors-servo merged commit 6be5218 into servo:master Feb 15, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@KiChjang KiChjang deleted the KiChjang:xhr-response-json branch Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@KiChjang @highfive @jdm @bors-servo @Ms2ger
You can’t perform that action at this time.