Closed Bug 1340334 Opened 3 years ago Closed 3 years ago

stylo: several tests crashes with "Caller should apply sibling hints"

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: xidorn, Assigned: emilio)

References

Details

Attachments

(1 file)

From autoland: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f6cde9d9294acbcbbd97eb18e0ee990f67a058ca&selectedJob=78111084

thread '<unnamed>' panicked at 'Caller should apply sibling hints', /home/worker/workspace/build/src/servo/components/style/data.rs:235
stack backtrace:
   1:     0x7f05a79979ea - std::sys::imp::backtrace::tracing::imp::write::h3188f035833a2635
   2:     0x7f05a79a5d4f - std::panicking::default_hook::{{closure}}::h6385b6959a2dd25b
   3:     0x7f05a79a594e - std::panicking::default_hook::he4f3b61755d7fa95
   4:     0x7f05a79a61f7 - std::panicking::rust_panic_with_hook::hf00b8130f73095ec
   5:     0x7f05a757876a - std::panicking::begin_panic::haa6e343a5b54e680
   6:     0x7f05a766d305 - <style::data::StoredRestyleHint as core::convert::From<style::restyle_hints::RestyleHint>>::from::h1fb6987b5d28088c
   7:     0x7f05a74ced68 - <T as core::convert::Into<U>>::into::ha2c96d381b187cf7
   8:     0x7f05a7536cc8 - Servo_NoteExplicitHints
   9:     0x7f05a5e01f09 - _ZN7mozilla19ServoRestyleManager16PostRestyleEventEPNS_3dom7ElementE13nsRestyleHint12nsChangeHint
  10:     0x7f05a5e0efc2 - _ZN7mozilla14RestyleManager14ContentRemovedEP7nsINodeP10nsIContentS4_
  11:     0x7f05a5e0f170 - _ZN7mozilla9PresShell14ContentRemovedEP11nsIDocumentP10nsIContentS4_iS4_
  12:     0x7f05a4ddc8e0 - _ZN11nsNodeUtils14ContentRemovedEP7nsINodeP10nsIContentiS3_
  13:     0x7f05a4ddc9af - _ZN7nsINode15doRemoveChildAtEjbP10nsIContentR19nsAttrAndChildArray
  14:     0x7f05a4cf6343 - _ZN7mozilla3dom17FragmentOrElement13RemoveChildAtEjb
  15:     0x7f05a4dc9ab9 - _ZN7nsINode11RemoveChildERS_RN7mozilla11ErrorResultE
  16:     0x7f05a4f3fdb6 - _ZN7mozilla3dom11NodeBindingL11removeChildEP9JSContextN2JS6HandleIP8JSObjectEEP7nsINodeRK19JSJitMethodCallArgs
  17:     0x7f05a53fb4cd - _ZN7mozilla3dom20GenericBindingMethodEP9JSContextjPN2JS5ValueE
  18:     0x7f05a6b1535e - _ZN2js12CallJSNativeEP9JSContextPFbS1_jPN2JS5ValueEERKNS2_8CallArgsE
  19:     0x7f05a6b28e36 - _ZN2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstructE
  20:     0x7f05a6b1cae4 - _ZL9InterpretP9JSContextRN2js8RunStateE
  21:     0x7f05a6b28a83 - _ZN2js9RunScriptEP9JSContextRNS_8RunStateE
  22:     0x7f05a6b2a940 - _ZN2js13ExecuteKernelEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectRKNS2_5ValueENS_16AbstractFramePtrEPS9_
  23:     0x7f05a6b2ac87 - _ZN2js7ExecuteEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectPNS2_5ValueE
  24:     0x7f05a6e9eba7 - _ZL8EvaluateP9JSContextN2js9ScopeKindEN2JS6HandleIP8JSObjectEERKNS3_22ReadOnlyCompileOptionsERNS3_18SourceBufferHolderENS3_13MutableHandleINS3_5ValueEEE
  25:     0x7f05a6e9f323 - _ZN2JS8EvaluateEP9JSContextRNS_16AutoObjectVectorERKNS_22ReadOnlyCompileOptionsERNS_18SourceBufferHolderENS_13MutableHandleINS_5ValueEEE
  26:     0x7f05a4dd8dfb - _ZN9nsJSUtils14EvaluateStringEP9JSContextRN2JS18SourceBufferHolderENS2_6HandleIP8JSObjectEERNS2_14CompileOptionsERKNS_15EvaluateOptionsENS2_13MutableHandleINS2_5ValueEEEPPv
  27:     0x7f05a4dd9152 - _ZN9nsJSUtils14EvaluateStringEP9JSContextRN2JS18SourceBufferHolderENS2_6HandleIP8JSObjectEERNS2_14CompileOptionsEPPv
  28:     0x7f05a4ded7b8 - _ZN14nsScriptLoader14EvaluateScriptEP19nsScriptLoadRequest.part.650
  29:     0x7f05a4dedb5a - _ZN14nsScriptLoader14ProcessRequestEP19nsScriptLoadRequest
  30:     0x7f05a4dfca93 - _ZN14nsScriptLoader22ProcessPendingRequestsEv
  31:     0x7f05a4dde1a2 - _ZN7mozilla6detail18RunnableMethodImplIP14nsScriptLoaderMS2_FvvELb1ELb0EIEE3RunEv
  32:     0x7f05a42650d8 - _ZN8nsThread16ProcessNextEventEbPb
  33:     0x7f05a4267237 - _Z19NS_ProcessNextEventP9nsIThreadb
  34:     0x7f05a45f287a - _ZN7mozilla3ipc11MessagePump3RunEPN4base11MessagePump8DelegateE
  35:     0x7f05a45d1a3e - _ZN11MessageLoop11RunInternalEv
  36:     0x7f05a45d1a65 - _ZN11MessageLoop3RunEv
  37:     0x7f05a5bf89e4 - _ZN14nsBaseAppShell3RunEv
  38:     0x7f05a69450aa - _ZN12nsAppStartup3RunEv
  39:     0x7f05a69bd20e - _ZN7XREMain11XRE_mainRunEv
  40:     0x7f05a69bda3d - _ZN7XREMain8XRE_mainEiPPcRKN7mozilla15BootstrapConfigE
  41:     0x7f05a69bdcc9 - _Z8XRE_mainiPPcRKN7mozilla15BootstrapConfigE
  42:           0x406115 - _ZL7do_mainiPPcS0_
  43:           0x4058da - main
  44:     0x7f05b0eae7ec - __libc_start_main
  45:           0x405b30 - <unknown>
Redirecting call to abort() to mozalloc_abort
layout/style/test/test_bug534804.html
layout/style/test/test_bug73586.html
Summary: stylo: test_bug534804.html crashes with "Caller should apply sibling hints" → stylo: several tests crashes with "Caller should apply sibling hints"
So this is because we send an explicit eRestyle_LaterSiblings to the restyle manager to handle the :empty change, which is plain wrong because we don't honor it.
I'm working on a patch for this.
Attached patch patchSplinter Review
Assignee: nobody → emilio+bugs
Status: NEW → ASSIGNED
Attachment #8842374 - Flags: review?(bobbyholley)
Blocks: 1343162
Note that we still do something that looks fishy to me, which is discarding hints if the element is not styled, which may be wrong if we post a LaterSiblings restyle hint.

I don't think right now we reach that though (given we look at the grandparent with flags that we set during selector matching in the only place we post those hints).
Blocks: 1342871
(In reply to Emilio Cobos Álvarez [:emilio] from comment #6)
> Here's a try run with the tests re-enabled:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=d9f9a398ad6d4d0efada6a186c9b924c6a067ee6

Note that this doesn't properly run the reftest runs right now (I'm complaining about this in bug 1340911, will try a fix soon). So for now you should probably do another run with |-b do -p linux64-stylo -u all -t none|.
Comment on attachment 8842374 [details] [diff] [review]
patch

Review of attachment 8842374 [details] [diff] [review]:
-----------------------------------------------------------------

Nice! r=me with a green reftest run.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5)
> Note that we still do something that looks fishy to me, which is discarding
> hints if the element is not styled, which may be wrong if we post a
> LaterSiblings restyle hint.

Yeah, that does seem dangerous. I think that Servo_NoteExplicitHints should check for LaterSiblings, and if the target node is unstyled, walk the later siblings and manually add Restyle_Self to any styled elements. Can you file a followup to do that?
Attachment #8842374 - Flags: review?(bobbyholley) → review+
(btw, I think those crashes you hit on your last try push are intermittents that I've seen before. I retriggered each a few times to check).
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #9)
> (btw, I think those crashes you hit on your last try push are intermittents
> that I've seen before. I retriggered each a few times to check).

Yeah, me too. I haven't kept an eye on the tree lately, but I believe we have a bug for that.

Anyway, servo PR: https://github.com/servo/servo/pull/15791

Thanks for the review Bobby! :)
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #8)
> Yeah, that does seem dangerous. I think that Servo_NoteExplicitHints should
> check for LaterSiblings, and if the target node is unstyled, walk the later
> siblings and manually add Restyle_Self to any styled elements. Can you file
> a followup to do that?

Did this ever get filed?
Flags: needinfo?(emilio+bugs)
Whoops, there you go: bug 1344626.

Also need to re-enable the tests.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: needinfo?(emilio+bugs)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.