Skip to content

Bring back run_with_memory_reporting in serviceworkerglobalscope#12557

Merged
bors-servo merged 1 commit into
servo:masterfrom
creativcoder:sw-scope
Aug 2, 2016
Merged

Bring back run_with_memory_reporting in serviceworkerglobalscope#12557
bors-servo merged 1 commit into
servo:masterfrom
creativcoder:sw-scope

Conversation

@creativcoder

@creativcoder creativcoder commented Jul 22, 2016 •
edited by larsbergstrom
Loading

Copy link
Copy Markdown
Contributor
  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • These changes do not require tests because its a refactor.

This change is Reviewable

@highfive highfive assigned ghost Jul 22, 2016
@highfive

highfive commented Jul 22, 2016

Copy link
Copy Markdown

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/serviceworkerglobalscope.rs

@highfive

highfive commented Jul 22, 2016

Copy link
Copy Markdown

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 22, 2016
@ghost

ghost commented Jul 22, 2016

Copy link
Copy Markdown

r? @jdm

@highfive highfive assigned jdm and unassigned ghost Jul 22, 2016
@nox nox assigned nox and unassigned jdm Jul 25, 2016
@nox nox added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 25, 2016
@nox

nox commented Jul 25, 2016

Copy link
Copy Markdown
Contributor

-S-awaiting-review +S-needs-code-changes

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

components/script/dom/serviceworkerglobalscope.rs, line 181 [r1] (raw file):

            });

            let reporter_name = format!("ServiceWorker-reporter-{}", random::<u64>());

The previous name for this reporter was service-worker-reporter, could you name it back like that and rename service-worker-reporter in components/script/dom/dedicatedworkerglobalscope.rs to dedicated-service-worker-reporter?

Thanks for your contribution!

Comments from Reviewable

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jul 25, 2016
@bors-servo

bors-servo commented Jul 27, 2016

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #12582) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Jul 27, 2016
@creativcoder creativcoder force-pushed the sw-scope branch 2 times, most recently from 79b1b12 to 8bb18ea Compare July 28, 2016 18:05
@creativcoder

creativcoder commented Jul 29, 2016

Copy link
Copy Markdown
Contributor Author

@nox this has been updated.

@nox nox removed the S-needs-rebase There are merge conflict errors. label Aug 1, 2016
@nox

nox commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

@bors-servo r+

@bors-servo

bors-servo commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

📌 Commit 4936fb2 has been approved by nox

@highfive 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 Aug 1, 2016
@bors-servo

bors-servo commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

⌛ Testing commit 4936fb2 with merge bf3135a...

bors-servo pushed a commit that referenced this pull request Aug 1, 2016
Auto merge of #12557 - creativcoder:sw-scope, r=nox
bf3135a 
Bring back run_with_memory_reporting in serviceworkerglobalscope

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [X] These changes do not require tests because its a refactor.

<!-- 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/12557)
<!-- Reviewable:end -->
@bors-servo

bors-servo commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

@bors-servo

bors-servo commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

@KiChjang

KiChjang commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

@bors-servo retry

  • wat

@bors-servo

bors-servo commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

⌛ Testing commit 4936fb2 with merge a008670...

bors-servo pushed a commit that referenced this pull request Aug 1, 2016
Auto merge of #12557 - creativcoder:sw-scope, r=nox
a008670 
Bring back run_with_memory_reporting in serviceworkerglobalscope

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [X] These changes do not require tests because its a refactor.

<!-- 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/12557)
<!-- Reviewable:end -->
@bors-servo

bors-servo commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

💔 Test failed - linux-rel

@highfive 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 Aug 1, 2016
@creativcoder

creativcoder commented Aug 2, 2016

Copy link
Copy Markdown
Contributor Author 
Ran 6801 tests finished in 818.0 seconds.
  • 6800 ran as expected. 1998 tests skipped.
  • 1 tests had unexpected subtest results

Tests with unexpected results:
  ▶ Unexpected subtest result in /html/browsers/history/the-history-interface/001.html:
  │ FAIL [expected PASS] pushState should not actually load the new URL
  │   → assert_true: expected true got undefined
  │ 
  │ tests12/<@http://web-platform.test:8000/html/browsers/history/the-history-interface/001.html:283:13
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1402:20
  │ test@http://web-platform.test:8000/resources/testharness.js:500:9
  └ tests12@http://web-platform.test:8000/html/browsers/history/the-history-interface/001.html:282:9

This seems unrelated to the changes made in this PR. @KiChjang Is that an intermittent ?

@KiChjang

KiChjang commented Aug 2, 2016

Copy link
Copy Markdown
Contributor

@bors-servo retry

bors-servo pushed a commit that referenced this pull request Aug 2, 2016
Auto merge of #12557 - creativcoder:sw-scope, r=nox
2aa257f 
Bring back run_with_memory_reporting in serviceworkerglobalscope

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

---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [X] These changes do not require tests because its a refactor.

<!-- 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/12557)
<!-- Reviewable:end -->
@bors-servo

bors-servo commented Aug 2, 2016

Copy link
Copy Markdown
Contributor

⌛ Testing commit 4936fb2 with merge 2aa257f...

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Aug 2, 2016
@bors-servo

bors-servo commented Aug 2, 2016

Copy link
Copy Markdown
Contributor

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev

@bors-servo bors-servo merged commit 4936fb2 into servo:master Aug 2, 2016
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Aug 2, 2016
@bors-servo bors-servo mentioned this pull request Aug 2, 2016
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nox nox

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@creativcoder @highfive @nox @bors-servo @KiChjang @jdm