Index the session past correctly when discarding. #14860

asajeffrey · 2017-01-05T12:59:21Z

Oops, indexed from the wrong end when discarding documents in #14312.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes do not require tests because we're not testing document discarding

This change is

asajeffrey · 2017-01-05T12:59:40Z

r? @cbrewster

emilio · 2017-01-05T13:00:55Z

Any chance to test this?

asajeffrey · 2017-01-05T13:03:27Z

Not entirely sure how we'd test this. Do we have a servo-specific wpt?

emilio · 2017-01-05T13:07:27Z

Yes, you have the tests/mozilla, and TestBinding if you need to expose internals.

emilio · 2017-01-05T13:08:07Z

I meant tests/wpt/mozilla/tests/

asajeffrey · 2017-01-05T13:09:37Z

Do we have tests that are only intended for servo, not for FF?

asajeffrey · 2017-01-05T13:11:35Z

May fix #7572?

emilio · 2017-01-05T13:13:02Z

Do we have tests that are only intended for servo, not for FF?

Yes, quite a few of them, grep for dom.testbinding.enabled.

asajeffrey · 2017-01-05T15:39:40Z

@emilio I added a test, took a while to work out how to test for this!

cbrewster

Shouldn't we be use an iframe for the test? Does it mess with the testharness to discard and reload the page?

asajeffrey · 2017-01-05T16:30:44Z

There's no messing with the test harness, just some hoop jumping to make sure the test reporting comes from a page with the same URL as the original.

cbrewster · 2017-01-05T17:01:05Z

@bors-servo r+

bors-servo · 2017-01-05T17:01:07Z

📌 Commit 0396934 has been approved by cbrewster

bors-servo · 2017-01-05T17:01:12Z

⌛ Testing commit 0396934 with merge 390985a...

…when-discarding, r=cbrewster Index the session past correctly when discarding.  Oops, indexed from the wrong end when discarding documents in #14312. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because we're not testing document discarding   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14860)

bors-servo · 2017-01-05T17:45:50Z

💔 Test failed - mac-rel-wpt1

cbrewster · 2017-01-05T17:47:24Z

  ▶ CRASH [expected OK] /html/browsers/origin/cross-origin-objects/cross-origin-objects-exceptions.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  │ 3.3 (Core Profile) Mesa 12.0.1
  │ called `Option::unwrap()` on a `None` value (thread ScriptThread PipelineId { namespace_id: PipelineNamespaceId(0), index: PipelineIndex(1) }, at /Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/src/libcore/option.rs:323)
  │ stack backtrace:
  │    0:        0x109dcd86e - backtrace::backtrace::trace::hc4ea27d646916501
  │    1:        0x109dcdb7c - backtrace::capture::Backtrace::new::hd6fb6e83f9059b0d
  │    2:        0x109b59874 - servo::main::{{closure}}::hfe009f9dcda553cc
  │    3:        0x10b6da0d3 - std::panicking::rust_panic_with_hook::hb2f44ed7df6389fd
  │    4:        0x10b6d9fa4 - std::panicking::begin_panic::h98e44359ade7bd0a
  │    5:        0x10b6d9ec2 - std::panicking::begin_panic_fmt::hb5af1180f1c36e1d
  │    6:        0x10b6d9e27 - rust_begin_unwind
  │    7:        0x10b7034e0 - core::panicking::panic_fmt::h37d320d6d0d8b0d4
  │    8:        0x10b7033e4 - core::panicking::panic::h350312d1589d7131
  │    9:        0x10a34f507 - script::dom::window::Window::parent::hf92f51f7f90699aa
  │   10:        0x10a34590d - <script::dom::window::Window as script::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods>::Parent::h6122ff9e91067e95
  │   11:        0x109fec218 - std::panicking::try::do_call::he71360b03e8a6bce
  │   12:        0x10b6dafca - __rust_maybe_catch_panic
  │   13:        0x10a62d240 - script::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::get_parent::h5eff038d7bc458d8
  │   14:        0x10a13d934 - script::dom::bindings::utils::generic_call::h4be311f4f448f4a3
  │   15:        0x10aae938e - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │   16:        0x10ab04e8c - _ZL10CallGetterP9JSContextN2JS6HandleIP8JSObjectEENS2_INS1_5ValueEEENS2_IPN2js5ShapeEEENS1_13MutableHandleIS6_EE
  │   17:        0x10aafecf0 - 2js25NativeGetExistingPropertyEP9JSContextN2JS6HandleIP8JSObjectEENS3_IPNS_12NativeObjectEEENS3_IPNS_5ShapeEEENS2_13MutableHandleINS2_5ValueEE
  │   18:        0x10ab0e879 - 2js9FetchNameILb0EEEbP9JSContextN2JS6HandleIP8JSObjectEES7_NS4_IPNS_12PropertyNameEEENS4_IPNS_5ShapeEEENS3_13MutableHandleINS3_5ValueEE
  │   19:        0x10aae32f4 - _ZL9InterpretP9JSContextRN2js8RunStateE
  │   20:        0x10aad626a - 2js9RunScriptEP9JSContextRNS_8RunState
  │   21:        0x10aaea221 - _ZN2js13ExecuteKernelEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectRKNS2_5ValueENS_16AbstractFramePtrEPS9_
  │   22:        0x10aa97c93 - _ZL10EvalKernelP9JSContextN2JS6HandleINS1_5ValueEEE8EvalTypeN2js16AbstractFramePtrENS2_IP8JSObjectEEPhNS1_13MutableHandleIS3_EE
  │   23:        0x10aa97364 - 2js12IndirectEvalEP9JSContextjPN2JS5Value
  │   24:        0x10aae938e - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │   25:        0x10aae9845 - 2js4CallEP9JSContextN2JS6HandleINS2_5ValueEEES5_RKNS_13AnyInvokeArgsENS2_13MutableHandleIS4_E
  │   26:        0x10aa8e5be - K2js7Wrapper4callEP9JSContextN2JS6HandleIP8JSObjectEERKNS3_8CallArgs
  │   27:        0x10aa80a2b - K2js23CrossCompartmentWrapper4callEP9JSContextN2JS6HandleIP8JSObjectEERKNS3_8CallArgs
  │   28:        0x10aa85e59 - 2js5Proxy4callEP9JSContextN2JS6HandleIP8JSObjectEERKNS3_8CallArgs
  │   29:        0x10aa867e9 - 2js10proxy_CallEP9JSContextjPN2JS5Value
  │   30:        0x10aae95ba - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │   31:        0x10aae2bac - _ZL9InterpretP9JSContextRN2js8RunStateE
  │   32:        0x10aad626a - 2js9RunScriptEP9JSContextRNS_8RunState
  │   33:        0x10aae96e2 - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │   34:        0x10aae9845 - 2js4CallEP9JSContextN2JS6HandleINS2_5ValueEEES5_RKNS_13AnyInvokeArgsENS2_13MutableHandleIS4_E
  │   35:        0x10aa15d70 - 2js9fun_applyEP9JSContextjPN2JS5Value
  │   36:        0x10aae938e - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │   37:        0x10aae2bac - _ZL9InterpretP9JSContextRN2js8RunStateE
  │   38:        0x10aad626a - 2js9RunScriptEP9JSContextRNS_8RunState
  │   39:        0x10aae96e2 - 2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstruct
  │   40:        0x10aae9845 - 2js4CallEP9JSContextN2JS6HandleINS2_5ValueEEES5_RKNS_13AnyInvokeArgsENS2_13MutableHandleIS4_E
  │   41:        0x10a9b5ff9 - _Z20JS_CallFunctionValueP9JSContextN2JS6HandleIP8JSObjectEENS2_INS1_5ValueEEERKNS1_16HandleValueArrayENS1_13MutableHandleIS6_EE
  │   42:        0x10a4b911a - script::dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull::Call::he83c80c52383cfca
  │   43:        0x10a4b8c92 - script::dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull::Call_::hba84ec2a9352088b
  │   44:        0x10a1ed7f3 - script::dom::eventdispatcher::handle_event::h18cf120475fd4a2b
  │   45:        0x10a1f007a - script::dom::eventdispatcher::invoke::hc2d21e11f5d6bc38
  │   46:        0x10a1ef2ac - script::dom::eventdispatcher::dispatch_event::hc36d51fb540f667e
  │   47:        0x10a1f99af - script::dom::eventtarget::EventTarget::fire_event_with_params::h7700405b4289f151
  │   48:        0x10a245293 - script::dom::htmliframeelement::HTMLIFrameElement::iframe_load_event_steps::hac9fb26764b1c44d
  │   49:        0x10a399a70 - script::script_thread::ScriptThread::handle_msg_from_constellation::h8d30a93e504ad522
  │   50:        0x10a397955 - script::script_thread::ScriptThread::handle_msgs::{{closure}}::h607d558afc76a27b
  │   51:        0x10a392831 - script::script_thread::ScriptThread::handle_msgs::h0625f27481ecd3b8
  │   52:        0x109f1a1c7 - std::panicking::try::do_call::h712b5dc69d667db4
  │   53:        0x10b6dafca - __rust_maybe_catch_panic
  │   54:        0x10a07cf0a - <F as alloc::boxed::FnBox<A>>::call_box::h9db941ec26154488
  │   55:        0x10b6d9044 - std::sys::imp::thread::Thread::new::thread_start::h0986e95fd3be13c5
  │   56:     0x7fff95aed059 - _pthread_body
  │   57:     0x7fff95aecfd6 - _pthread_start
  │ ERROR:servo: called `Option::unwrap()` on a `None` value
  └ Pipeline failed in hard-fail mode.  Crashing!

asajeffrey · 2017-01-05T20:39:47Z

IRC chat with @cbrewster http://logs.glob.uno/?c=mozilla%23servo&s=5+Jan+2017&e=5+Jan+2017#c587879

TL;DR the crash is the same as #14312 (comment), with root cause #14843. The long-term solution is to fix that issue, but in the short term increasing the default --max-session-history to 20 is a work-around.

@bors-servo r=cbrewster

bors-servo · 2017-01-05T20:39:48Z

📌 Commit 53436c8 has been approved by cbrewster

bors-servo · 2017-01-05T21:22:34Z

⌛ Testing commit 53436c8 with merge 2bc27d8...

…when-discarding, r=cbrewster Index the session past correctly when discarding.  Oops, indexed from the wrong end when discarding documents in #14312. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because we're not testing document discarding   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14860)

bors-servo · 2017-01-05T22:00:48Z

💔 Test failed - linux-rel-wpt

jdm · 2017-01-05T22:04:55Z

  ▶ TIMEOUT [expected OK] /_mozilla/mozilla/servo-max-session-history.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  └ 3.3 (Core Profile) Mesa 12.0.1

asajeffrey · 2017-01-05T23:11:34Z

Oh that's ironic, that would be the test I just added to make sure that document discarding is happening, and which assumes the default max session history is 16.

asajeffrey · 2017-01-06T11:06:07Z

Fixed the test. @bors-servo r=cbrewster

bors-servo · 2017-01-06T11:06:08Z

📌 Commit b401556 has been approved by cbrewster

bors-servo · 2017-01-06T11:53:37Z

⌛ Testing commit b401556 with merge 7f9fd0e...

…when-discarding, r=cbrewster Index the session past correctly when discarding.  Oops, indexed from the wrong end when discarding documents in #14312. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because we're not testing document discarding   --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14860)

bors-servo · 2017-01-06T13:15:08Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev

asajeffrey added the A-constellation Involves the constellation label Jan 5, 2017

highfive assigned emilio Jan 5, 2017

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 5, 2017

highfive assigned cbrewster and unassigned emilio Jan 5, 2017

asajeffrey mentioned this pull request Jan 5, 2017

Servo panics exactly after 144 navigations #7572

Closed

cbrewster reviewed Jan 5, 2017

View reviewed changes

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 Jan 5, 2017

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 Jan 5, 2017

Alan Jeffrey added 2 commits January 5, 2017 20:36

Increased default max-session-history to 20.

3e1a242

Index the session past correctly when discarding.

5fd284c

asajeffrey force-pushed the constellation-index-past-correctly-when-discarding branch from 0396934 to 53436c8 Compare January 5, 2017 20:36

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 5, 2017

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 Jan 5, 2017

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 Jan 5, 2017

Added a test to make sure that forward-then-back triggers a reload.

b401556

asajeffrey force-pushed the constellation-index-past-correctly-when-discarding branch from 53436c8 to b401556 Compare January 6, 2017 10:59

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 6, 2017

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 Jan 6, 2017

bors-servo merged commit b401556 into servo:master Jan 6, 2017

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index the session past correctly when discarding. #14860

Index the session past correctly when discarding. #14860

asajeffrey commented Jan 5, 2017 •

edited by larsbergstrom

asajeffrey commented Jan 5, 2017

emilio commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

emilio commented Jan 5, 2017

emilio commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

emilio commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

cbrewster left a comment

asajeffrey commented Jan 5, 2017

cbrewster commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

cbrewster commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

jdm commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

asajeffrey commented Jan 6, 2017

bors-servo commented Jan 6, 2017

bors-servo commented Jan 6, 2017

bors-servo commented Jan 6, 2017

Index the session past correctly when discarding. #14860

Index the session past correctly when discarding. #14860

Conversation

asajeffrey commented Jan 5, 2017 • edited by larsbergstrom

asajeffrey commented Jan 5, 2017

emilio commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

emilio commented Jan 5, 2017

emilio commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

emilio commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

cbrewster left a comment

Choose a reason for hiding this comment

asajeffrey commented Jan 5, 2017

cbrewster commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

cbrewster commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

bors-servo commented Jan 5, 2017

jdm commented Jan 5, 2017

asajeffrey commented Jan 5, 2017

asajeffrey commented Jan 6, 2017

bors-servo commented Jan 6, 2017

bors-servo commented Jan 6, 2017

bors-servo commented Jan 6, 2017

asajeffrey commented Jan 5, 2017 •

edited by larsbergstrom