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

When webdriver is getting a pipeline id, it should wait for the pipeline document to be ready #11140

Merged

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented May 11, 2016

Thank you for contributing to Servo! Please add an X inside each [ ] when the step is complete, and replace __ with appropriate data:

  • ./mach build does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #11117 (github issue number if applicable).

Either:

  • There are tests for these changes OR
  • These changes do not require tests because they only impact webdriver

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


This change is Reviewable

…ine document to be ready.
@highfive
Copy link

highfive commented May 11, 2016

Heads up! This PR modifies the following files:

  • @jgraham: components/webdriver_server/lib.rs
@asajeffrey
Copy link
Member Author

asajeffrey commented May 11, 2016

@highfive highfive assigned jgraham and unassigned wafflespeanut May 11, 2016
@jgraham
Copy link
Contributor

jgraham commented May 11, 2016

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


components/compositing/constellation.rs, line 1397 [r1] (raw file):

            .and_then(|frame_id| self.frames.get(&frame_id))
            .map(|frame| frame.current);
        let current_pipeline_id_loaded = current_pipeline_id

I think you should just merge this into the previous line like .map(|frame| (frame.current, true))


Comments from Reviewable

@asajeffrey
Copy link
Member Author

asajeffrey commented May 11, 2016

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


components/compositing/constellation.rs, line 1397 [r1] (raw file):

Previously, jgraham wrote…

I think you should just merge this into the previous line like .map(|frame| (frame.current, true))


We use current_pipeline_id later on in .find(|x| x.old_pipeline_id == current_pipeline_id).


Comments from Reviewable

@jgraham
Copy link
Contributor

jgraham commented May 12, 2016

Review status: all files reviewed at latest revision, all discussions resolved.


components/compositing/constellation.rs, line 1397 [r1] (raw file):

Previously, asajeffrey (Alan Jeffrey) wrote…

We use current_pipeline_id later on in .find(|x| x.old_pipeline_id == current_pipeline_id).


Well I can't read. I guess that doesn't preclude it, but sure, this is fine.


Comments from Reviewable

@jgraham
Copy link
Contributor

jgraham commented May 12, 2016

@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2016

📌 Commit 00a8efe has been approved by jgraham

@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2016

Testing commit 00a8efe with merge b3c15fd...

bors-servo added a commit that referenced this pull request May 12, 2016
…t-ready, r=jgraham

When webdriver is getting a pipeline id, it should wait for the pipeline document to be ready

Thank you for contributing to Servo! Please add an `X` inside each `[ ]` when the step is complete, and replace `__` with appropriate data:
- [X] `./mach build` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11117 (github issue number if applicable).

Either:
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because they only impact webdriver

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="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11140)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2016

💔 Test failed - linux-rel

@highfive
Copy link

highfive commented May 12, 2016

  ▶ TIMEOUT [expected PASS] /css21_dev/html4/border-bottom-color-028.htm
  │ 
  │ Xlib:  extension &#34;XFree86-VidModeExtension&#34; missing on display &#34;:0&#34;.
  │ Shutting down the Constellation after generating an output file or exit flag specified
  │ thread &#39;Constellation&#39; panicked at &#39;called `Result::unwrap()` on an `Err` value: IoError(Error { repr: Os { code: 104, message: &#34;Connection reset by peer&#34; } })&#39;, ../src/libcore/result.rs:785
  │ stack backtrace:
  │    1:     0x7f856eae66a0 - std::sys::backtrace::tracing::imp::write::h9fb600083204ae7f
  │    2:     0x7f856eaede1b - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::hca543c34f11229ac
  │    3:     0x7f856eaeda83 - std::panicking::default_hook::hc2c969e7453d080c
  │    4:     0x7f856e24c287 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::h625edf9a36237d31
  │    5:     0x7f856ead3b63 - std::sys_common::unwind::begin_unwind_inner::h30e12d15ce2b2e25
  │    6:     0x7f856ead55a8 - std::sys_common::unwind::begin_unwind_fmt::hb2de8a9968d38523
  │    7:     0x7f856eae58f1 - rust_begin_unwind
  │    8:     0x7f856eb24e9f - core::panicking::panic_fmt::h257ceb0aa351d801
  │    9:     0x7f856dd05cce - core::result::unwrap_failed::h330560dbaf0a51e7
  │   10:     0x7f856dd2bfce - gfx::font_cache_thread::FontCacheThread::exit::ha2f1f4fd6ebc6397
  │   11:     0x7f856d0ebd28 - _&lt;compositing..Constellation&lt;LTF, STF&gt;&gt;::handle_exit::hae6d0417398a20fa
  │   12:     0x7f856d0c15c2 - _&lt;compositing..Constellation&lt;LTF, STF&gt;&gt;::handle_request::h3c97e2d5b7782802
  │   13:     0x7f856d0b520c - std::sys_common::unwind::t</span><span class="stdout">ry::try_fn::h12e8cdd1c52fbc04
  │   14:     0x7f856eae587b - __rust_try
  │   15:     0x7f856eae580d - std::sys_common::unwind::inner_try::h47a4d9cd4a369dcd
  │   16:     0x7f856d0b67fa - _&lt;F as std..boxed..FnBox&lt;A&gt;&gt;::call_box::h4fbd446f73dda14f
  │   17:     0x7f856eaec2d4 - std::sys::thread::Thread::new::thread_start::h6f266e069bf4ec2b
  │   18:     0x7f856aacc181 - start_thread
  │   19:     0x7f856a5e347c - __clone
  └   20:                0x0 - &lt;unknown&gt;
@jgraham
Copy link
Contributor

jgraham commented May 12, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2016

Testing commit 00a8efe with merge 1a834b5...

bors-servo added a commit that referenced this pull request May 12, 2016
…t-ready, r=jgraham

When webdriver is getting a pipeline id, it should wait for the pipeline document to be ready

Thank you for contributing to Servo! Please add an `X` inside each `[ ]` when the step is complete, and replace `__` with appropriate data:
- [X] `./mach build` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #11117 (github issue number if applicable).

Either:
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because they only impact webdriver

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="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11140)
<!-- Reviewable:end -->
@Ms2ger
Copy link
Contributor

Ms2ger commented May 12, 2016

That was #8815

@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2016

@bors-servo bors-servo merged commit 00a8efe into servo:master May 12, 2016
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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

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