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

Freeze old pipeline in iframes #8886

Merged
merged 3 commits into from Dec 18, 2015

Conversation

Projects
None yet
7 participants
@paulrouget
Contributor

paulrouget commented Dec 8, 2015

Fixes #8673 and part of #8674

Review on Reviewable

@paulrouget

This comment has been minimized.

Contributor

paulrouget commented Dec 8, 2015

Could someone tell me where I should write a test?

@Ms2ger

This comment has been minimized.

Contributor

Ms2ger commented Dec 8, 2015

Sorry, wrong button.

Add a test under tests/wpt/mozilla? If you're using mozbrowser, maybe create a new subdirectory for it. You may need to set prefs, these are in the metadata directory

@Ms2ger Ms2ger reopened this Dec 8, 2015

@paulrouget

This comment has been minimized.

Contributor

paulrouget commented Dec 9, 2015

I didn't manage to properly test the frozen state of the iframe. This test crashes without this patch though.

@Manishearth

This comment has been minimized.

Member

Manishearth commented Dec 16, 2015

@mbrubeck

This comment has been minimized.

Contributor

mbrubeck commented Dec 16, 2015

Reviewed 1 of 1 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from the review on Reviewable.io

@mbrubeck

This comment has been minimized.

Contributor

mbrubeck commented Dec 16, 2015

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 16, 2015

📌 Commit 0da33f9 has been approved by mbrubeck

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 16, 2015

⌛️ Testing commit 0da33f9 with merge 3ae8a87...

bors-servo added a commit that referenced this pull request Dec 16, 2015

Auto merge of #8886 - paulrouget:freeze-pipeline-iframe, r=mbrubeck
Freeze old pipeline in iframes

Fixes #8673 and part of #8674

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

This comment has been minimized.

Contributor

bors-servo commented Dec 16, 2015

💔 Test failed - mac-rel-wpt

@mbrubeck

This comment has been minimized.

Contributor

mbrubeck commented Dec 16, 2015

  ▶ CRASH [expected TIMEOUT] /html/browsers/browsing-the-web/navigating-across-documents/002.html
  │ 
  │ thread 'ScriptTask PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }' panicked at 'ScriptTask: received freeze msg for a
  │                     pipeline ID not associated with this script task. This is a bug.', ../src/libcore/option.rs:335
  │ stack backtrace:
  │    1:        0x10ea2d178 - sys::backtrace::tracing::imp::write::hca66179add43b2e5lLt
  │    2:        0x10ea2f23f - panicking::log_panic::_<closure>::closure.40842
  │    3:        0x10ea2ece2 - panicking::log_panic::h7a04edac7e1f2797HEx
  │    4:        0x10ea1aaf6 - sys_common::unwind::begin_unwind_inner::h39a94087039ae0efnOs
  │    5:        0x10ea1aede - sys_common::unwind::begin_unwind_fmt::h7be0824d83765cd5tNs
  │    6:        0x10ea2c797 - rust_begin_unwind
  │    7:        0x10ea518a0 - panicking::panic_fmt::h871c0ef100fda9e9LFK
  │    8:        0x10dc99bc8 - script_task::_<impl>::handle_msg_from_constellation::h48e9f2652a337bc5D02
  │    9:        0x10dc9539e - script_task::_<impl>::handle_msgs::_<closure>::closure.158717
  │   10:        0x10dc7e39d - script_task::_<impl>::handle_msgs::h09234e46a6f77ec4LP2
  │   11:        0x10dc46017 - sys_common::unwind::try::try_fn::try_fn::h14806146711523557709
  │   12:        0x10ea2c5b8 - __rust_try
  │   13:        0x10ea296be - sys_common::unwind::try::inner_try::h819d2da8e3dc4516VKs
  │   14:        0x10dc47a61 - boxed::_<impl>::call_box::call_box::h13042718455976863970
  │   15:        0x10ea2e4cd - sys::thread::_<impl>::new::thread_start::h328957c252b95d3cOYw
  │   16:     0x7fff84ed8059 - _pthread_body
  │   17:     0x7fff84ed7fd6 - _pthread_start
  └ Pipeline failed in hard-fail mode.  Crashing!
@paulrouget

This comment has been minimized.

Contributor

paulrouget commented Dec 17, 2015

The issue was that the page was getting a freeze message while it was not created yet. Latest commit addresses this issue.

@jdm

This comment has been minimized.

Member

jdm commented Dec 17, 2015

@bors-servo: r+
That solution looks good to me. Thanks!

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 17, 2015

📌 Commit 47a7d17 has been approved by jdm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 17, 2015

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

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 18, 2015

🔒 Merge conflict

@paulrouget paulrouget force-pushed the paulrouget:freeze-pipeline-iframe branch from 47a7d17 to f0b25e1 Dec 18, 2015

@Manishearth

This comment has been minimized.

Member

Manishearth commented Dec 18, 2015

@bors-servo delegate+ r=mbrubeck,jdm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 18, 2015

📌 Commit f0b25e1 has been approved by mbrubeck,jdm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 18, 2015

✌️ @paulrouget can now approve this pull request

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Dec 18, 2015

⌛️ Testing commit f0b25e1 with merge 2ef972b...

bors-servo added a commit that referenced this pull request Dec 18, 2015

Auto merge of #8886 - paulrouget:freeze-pipeline-iframe, r=mbrubeck,jdm
Freeze old pipeline in iframes

Fixes #8673 and part of #8674

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

This comment has been minimized.

Contributor

bors-servo commented Dec 18, 2015

@bors-servo bors-servo merged commit f0b25e1 into servo:master Dec 18, 2015

2 of 3 checks passed

code-review/reviewable Review in progress: 1 of 4 files reviewed, all discussions resolved
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