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

iframe.goBack() panics if called too early #8674

Closed
paulrouget opened this issue Nov 25, 2015 · 15 comments
Closed

iframe.goBack() panics if called too early #8674

paulrouget opened this issue Nov 25, 2015 · 15 comments

Comments

@paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Nov 25, 2015

<iframe mozbrowser="true" src="data:text/html,<script>location.assign('data:,x')</script>"></iframe>
<script>
  var iframe = document.querySelector("iframe");
  iframe.addEventListener("mozbrowserloadend", () => iframe.goBack());
</script>
thread 'ScriptTask PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(0) }' panicked at 'assertion failed: self.suspended_since.get().is_some()', /Users/paul/git/servo/components/script/timers.rs:349
stack backtrace:
   1:        0x1041ec728 - sys::backtrace::tracing::imp::write::h662793599f713c74T5s
   2:        0x1041ee90f - panicking::log_panic::_<closure>::closure.38923
   3:        0x1041ee3b9 - panicking::log_panic::h87af6096e0b6fa7c4Uw
   4:        0x1041da0d6 - sys_common::unwind::begin_unwind_inner::h1e338bc64cf56d9dr8r
   5:        0x10237e147 - sys_common::unwind::begin_unwind::begin_unwind::h442923392230891188
   6:        0x102b9bcd7 - timers::_<impl>::resume::hd7c50b6547582733YW1
   7:        0x102b9bbdb - dom::window::_<impl>::thaw::h6dcbc1ebb597eb086cV
   8:        0x102c89100 - script_task::_<impl>::handle_thaw_msg::hda0bd12aaf56888eX2Z
   9:        0x102c8612c - script_task::_<impl>::handle_msg_from_constellation::h4b19eddbc2adee90GCZ
  10:        0x102c84961 - script_task::_<impl>::handle_msgs::_<closure>::closure.157149
  11:        0x102c84552 - script_task::_<impl>::profile_event::h9736138210224090807
  12:        0x102c57239 - script_task::_<impl>::handle_msgs::h3a5fe9faeff19152nqZ
  13:        0x102c03e00 - script_task::_<impl>::start::h03d6119e050f589ddqZ
  14:        0x102c03db4 - script_task::_<impl>::create::_<closure>::_<closure>::closure.155887
  15:        0x102c03b20 - mem::_<impl>::run_with_memory_reporting::run_with_memory_reporting::h673928561397631059
  16:        0x102bef68f - script_task::_<impl>::create::_<closure>::closure.155291
  17:        0x102beec77 - util::task::spawn_named_with_send_on_failure::_<closure>::closure.155284
  18:        0x102beebc7 - boxed::_<impl>::call_box::call_box::h15694691246183954468
  19:        0x1023ad49b - boxed::_<impl>::call_once::call_once::h4242493320255577343
  20:        0x1023ad06d - std::thread::_<impl>::spawn_inner::_<closure>::_<closure>::closure.116136
  21:        0x1023ad019 - sys_common::unwind::try::try_fn::try_fn::h4943420568539498126
  22:        0x1041ebbf8 - __rust_try
  23:        0x1041e909e - sys_common::unwind::try::inner_try::hc13d8e198528cd0fZ4r
  24:        0x1023acf83 - sys_common::unwind::try::try::h16910258665589907076
  25:        0x1023acdfb - std::thread::_<impl>::spawn_inner::_<closure>::closure.116112
  26:        0x1023ad71c - boxed::_<impl>::call_box::call_box::h11799493626222205043
  27:        0x1041edcdd - sys::thread::_<impl>::new::thread_start::h906ddce3a93d4852ksw
  28:     0x7fff98438c12 - _pthread_body
  29:     0x7fff98438b8f - _pthread_start
thread 'Constellation' panicked at 'unable to find pipeline - this is a bug', src/libcore/option.rs:333
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Nov 25, 2015

subpage_map:

(parent: PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }, subpage: SubpageId(0)), pipeline: PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(0) }
(parent: PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }, subpage: SubpageId(1)), pipeline: PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1) }

pipeline_to_frame_map:

PipelineId { namespace_id: PipelineNamespaceId(1), index: PipelineIndex(0) }, pipeline id: FrameId(1)
PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(0) }, pipeline id: FrameId(0)

Here we have a pipeline (namespace_id: PipelineNamespaceId(1), index: PipelineIndex(1)) with no associated frame, and it happens to be the one use in handle_navigate_msg.

We would expect add_or_replace_pipeline_in_frame_tree to be called before it's possible to call goBack.

I guess it's just enough to bail if there's no associated frame then.

@paulrouget paulrouget changed the title iframe.goBack() panics iframe.goBack() panics if page not fully loaded yet Nov 25, 2015
@paulrouget paulrouget changed the title iframe.goBack() panics if page not fully loaded yet iframe.goBack() panics if called too early Nov 25, 2015
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Nov 25, 2015

I guess it's just enough to bail if there's no associated frame then.

Not enough. More panics follow.

@jdm
Copy link
Member

@jdm jdm commented Nov 25, 2015

Note that there was actually an earlier panic reported than the constellation one in the original log.

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Nov 25, 2015

Hm, I don't get always the same panic first.

Either:

Or:

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Nov 27, 2015

If add_or_replace_pipeline_in_frame_tree is call before handle_navigate_msg, the iframe is found, but then it panics in pipeline.thaw() in the same function (pipeline is not frozen at this point).

If it's not, then it panics because unwrapping iframe_id which is None.

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Nov 27, 2015

If I understand correctly:

goBack() panics because self.subpage_map and self.pipeline_to_frame_map contains conflicting information.

handle_navigate_msg ask for subpage(0), which is associated to PipelineId {namespace_id:1, index:0}, which is associated to no iframe.

The iframe is associated to PipelineId { namespace_id:0, index:0}, which is not inside the iframe, but its parent document.

So my guess is that pipeline_to_frame_map is wrong.

handle_painter_ready_msg is in charge of updating pipeline_to_frame_map.

@jdm
Copy link
Member

@jdm jdm commented Nov 27, 2015

I wonder if we could fix this by removing all traces of subpages from code. I believe they're unnecessary since we introduced pipeline namespaces.

@jdm
Copy link
Member

@jdm jdm commented Nov 27, 2015

Also if this is to do with handle_painter_ready_msg, #8612 may affect this.

@rastus-vernon
Copy link

@rastus-vernon rastus-vernon commented Nov 28, 2015

I was able to reproduce these panic messages with the following steps:

  1. Run ./mach run --release https://github.com/servo/servo.
  2. Click on the link to servo.org.
  3. Press Backspace to go back in the history.
  4. Press Shift+Backspace to go forward in the history.
@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Nov 28, 2015

Could be related to #8461

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Dec 8, 2015

With #8612 - we hit the pipeline.thaw() panic first every time.

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Dec 8, 2015

It panics because when an iframe navigates to a new URL, the document is not frozen (not same code path as non subpage pipelines).

@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Dec 8, 2015

#8886 addresses the thaw() panic. With #8886 and #8612, we end up with the missing pipeline panic described in #8674 (comment)

bors-servo added a commit that referenced this issue Dec 16, 2015
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 added a commit that referenced this issue Dec 18, 2015
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 -->
@paulrouget
Copy link
Contributor Author

@paulrouget paulrouget commented Dec 18, 2015

Panic is gone.

@paulrouget paulrouget closed this Dec 18, 2015
jrmuizel pushed a commit to jrmuizel/gecko-cinnabar that referenced this issue Jun 12, 2017
…freeze-pipeline-iframe); r=mbrubeck,jdm

Fixes servo/servo#8673 and part of servo/servo#8674

Source-Repo: https://github.com/servo/servo
Source-Revision: 2ef972b53bc4ff3783a60bfc45a52beee3065e97
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…freeze-pipeline-iframe); r=mbrubeck,jdm

Fixes servo/servo#8673 and part of servo/servo#8674

Source-Repo: https://github.com/servo/servo
Source-Revision: 2ef972b53bc4ff3783a60bfc45a52beee3065e97

UltraBlame original commit: 715d5bf1ac9d8a947d96f3e71afd0376c7654fab
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…freeze-pipeline-iframe); r=mbrubeck,jdm

Fixes servo/servo#8673 and part of servo/servo#8674

Source-Repo: https://github.com/servo/servo
Source-Revision: 2ef972b53bc4ff3783a60bfc45a52beee3065e97

UltraBlame original commit: 715d5bf1ac9d8a947d96f3e71afd0376c7654fab
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…freeze-pipeline-iframe); r=mbrubeck,jdm

Fixes servo/servo#8673 and part of servo/servo#8674

Source-Repo: https://github.com/servo/servo
Source-Revision: 2ef972b53bc4ff3783a60bfc45a52beee3065e97

UltraBlame original commit: 715d5bf1ac9d8a947d96f3e71afd0376c7654fab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.