1365871 - stylo: some editor/libeditor/tests/ crashes with MOZ_CRASH(stylo: not implemented) at /Users/ferjm/dev/mozilla/mozilla-central/editor/libeditor/CSSEditUtils.cpp:553

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Fernando Jiménez Moreno [:ferjm]]

Affected tests:

* editor/libeditor/tests/browserscope/test_richtext2.html 
* editor/libeditor/tests/browserscope/test_richtext.html 
* editor/libeditor/tests/test_bug290026.html

Stack trace:

GECKO(21810) | Hit MOZ_CRASH(stylo: not implemented) at /Users/ferjm/dev/mozilla/mozilla-central/editor/libeditor/CSSEditUtils.cpp:553
GECKO(21810) | #01: mozilla::HTMLEditRules::ChangeIndentation(mozilla::dom::Element&, mozilla::HTMLEditRules::Change) (nsCOMPtr.h:427, in XUL)
GECKO(21810) | #02: mozilla::HTMLEditRules::WillCSSIndent(mozilla::dom::Selection*, bool*, bool*) (nsCOMPtr.h:374, in XUL)
GECKO(21810) | #03: mozilla::HTMLEditRules::WillIndent(mozilla::dom::Selection*, bool*, bool*) (HTMLEditRules.cpp:3683, in XUL)
GECKO(21810) | #04: mozilla::HTMLEditRules::WillDoAction(mozilla::dom::Selection*, mozilla::RulesInfo*, bool*, bool*) (HTMLEditRules.cpp:657, in XUL)
GECKO(21810) | #05: mozilla::HTMLEditor::Indent(nsAString const&) (HTMLEditor.cpp:2206, in XUL)
GECKO(21810) | #06: nsIndentCommand::DoCommand(char const*, nsISupports*) (nsComposerCommands.cpp:466, in XUL)
GECKO(21810) | #07: nsControllerCommandTable::DoCommand(char const*, nsISupports*) (nsControllerCommandTable.cpp:147, in XUL)
GECKO(21810) | #08: nsBaseCommandController::DoCommand(char const*) (nsBaseCommandController.cpp:136, in XUL)
GECKO(21810) | #09: nsCommandManager::DoCommand(char const*, nsICommandParams*, mozIDOMWindowProxy*) (nsCommandManager.cpp:214, in XUL)
GECKO(21810) | #10: nsHTMLDocument::ExecCommand(nsAString const&, bool, nsAString const&, nsIPrincipal&, mozilla::ErrorResult&) (ErrorResult.h:376, in XUL)
GECKO(21810) | #11: mozilla::dom::HTMLDocumentBinding::execCommand(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) (HTMLDocumentBinding.cpp:835, in XUL)
GECKO(21810) | #12: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (BindingUtils.cpp:2956, in XUL)
GECKO(21810) | #13: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:294, in XUL)
GECKO(21810) | #14: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:470, in XUL)
GECKO(21810) | #15: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(21810) | #16: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(21810) | #17: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(21810) | #18: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (Interpreter.cpp:534, in XUL)
GECKO(21810) | #19: js::fun_apply(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1288, in XUL)
GECKO(21810) | #20: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:294, in XUL)
GECKO(21810) | #21: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:470, in XUL)
GECKO(21810) | #22: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(21810) | #23: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(21810) | #24: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(21810) | #25: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (Interpreter.cpp:534, in XUL)
GECKO(21810) | #26: JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (jsapi.cpp:2891, in XUL)
GECKO(21810) | #27: mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) (EventHandlerBinding.cpp:260, in XUL)
GECKO(21810) | #28: void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) (EventHandlerBinding.h:362, in XUL)
GECKO(21810) | #29: mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) (JSEventHandler.cpp:215, in XUL)
GECKO(21810) | #30: mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) (EventListenerManager.cpp:1146, in XUL)
GECKO(21810) | #31: mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) (EventListenerManager.cpp:1320, in XUL)
GECKO(21810) | #32: mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) (EventDispatcher.cpp:319, in XUL)
GECKO(21810) | #33: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) (EventDispatcher.cpp:464, in XUL)
GECKO(21810) | #34: mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) (EventDispatcher.cpp:828, in XUL)
GECKO(21810) | #35: nsDocumentViewer::LoadComplete(nsresult) (nsDocumentViewer.cpp:1074, in XUL)
GECKO(21810) | #36: nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) (nsDocShell.cpp:7680, in XUL)
GECKO(21810) | #37: nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) (nsCOMPtr.h:402, in XUL)
GECKO(21810) | #38: non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) (nsDocShell.cpp:7370, in XUL)
GECKO(21810) | #39: nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) (nsDocLoader.cpp:1276, in XUL)
GECKO(21810) | #40: nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) (nsDocLoader.cpp:859, in XUL)
GECKO(21810) | #41: nsDocLoader::DocLoaderIsEmpty(bool) (nsDocLoader.cpp:749, in XUL)
GECKO(21810) | #42: nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) (nsCOMPtr.h:402, in XUL)
GECKO(21810) | #43: non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) (nsDocLoader.cpp:487, in XUL)
GECKO(21810) | #44: mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) (nsLoadGroup.cpp:629, in XUL)
GECKO(21810) | #45: nsDocument::DoUnblockOnload() (nsCOMPtr.h:402, in XUL)
GECKO(21810) | #46: nsDocument::DispatchContentLoadedEvents() (nsCOMPtr.h:402, in XUL)
GECKO(21810) | #47: mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, false>::Run() (nsThreadUtils.h:958, in XUL)
GECKO(21810) | #48: mozilla::SchedulerGroup::Runnable::Run() (SchedulerGroup.cpp:370, in XUL)
GECKO(21810) | #49: nsThread::ProcessNextEvent(bool, bool*) (Maybe.h:224, in XUL)
GECKO(21810) | #50: NS_ProcessPendingEvents(nsIThread*, unsigned int) (nsThreadUtils.cpp:335, in XUL)
GECKO(21810) | #51: nsBaseAppShell::NativeEventCallback() (nsBaseAppShell.cpp:98, in XUL)
GECKO(21810) | #52: nsAppShell::ProcessGeckoEvents(void*) (nsAppShell.mm:400, in XUL)
GECKO(21810) | #53: CFStringFold (in CoreFoundation) + 4209
GECKO(21810) | #54: -[_CFXNotificationRegistrar match:object:observer:enumerator:] (in CoreFoundation) + 1373
GECKO(21810) | #55: -[NSTaggedPointerString uppercaseStringWithLocale:] (in CoreFoundation) + 166
GECKO(21810) | #56: -[__NSDictionaryI countByEnumeratingWithState:objects:count:] (in CoreFoundation) + 100
GECKO(21810) | #57: RunCurrentEventLoopInMode (in HIToolbox) + 240
GECKO(21810) | #58: ReceiveNextEventCommon (in HIToolbox) + 432
GECKO(21810) | #59: _BlockUntilNextEventMatchingListInModeWithFilter (in HIToolbox) + 71
GECKO(21810) | #60: _DPSNextEvent (in AppKit) + 1120
GECKO(21810) | #61: -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (in AppKit) + 2796
GECKO(21810) | #62: -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (nsAppShell.mm:130, in XUL)
GECKO(21810) | #63: -[NSApplication run] (in AppKit) + 926
GECKO(21810) | #64: nsAppShell::Run() (nsCOMPtr.h:551, in XUL)
GECKO(21810) | #65: XRE_RunAppShell() (nsEmbedFunctions.cpp:893, in XUL)
GECKO(21810) | #66: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (MessagePump.cpp:269, in XUL)
GECKO(21810) | #67: MessageLoop::Run() (message_loop.cc:505, in XUL)
GECKO(21810) | #68: XRE_InitChildProcess(int, char**, XREChildData const*) (nsEmbedFunctions.cpp:713, in XUL)
GECKO(21810) | #69: main (plugin-container.cpp:64, in plugin-container)

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Bug 1365871 - stylo: Make editor/libeditor/tests pass by removing MOZ_CRASH from CSSEditUtils.

The MOZ_CRASH was added because we were lacking a implementation of ServoDeclarationBlock::GetPropertyValueByID, but it was added in Bug1294299

Review commit: https://reviewboard.mozilla.org/r/142258/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/142258/

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8870760 [details]
Bug 1365871 - stylo: Make editor/libeditor/tests pass by removing MOZ_CRASH from CSSEditUtils.

https://reviewboard.mozilla.org/r/142258/#review146222

r=me with the issue addressed.

::: editor/libeditor/CSSEditUtils.cpp:558
(Diff revision 1)
> +  if (decl->IsServo()) {
> +    decl->AsServo()->GetPropertyValueByID(prop, aValue);
> +  } else {
> -  decl->AsGecko()->GetPropertyValueByID(prop, aValue);
> +    decl->AsGecko()->GetPropertyValueByID(prop, aValue);
> +  }

You should be able to do `decl->GetPropertyValueByID` directly, which will give you want you want: https://dxr.mozilla.org/mozilla-central/rev/37d777d872002597198405f22eefa9199e5103af/layout/style/DeclarationBlock.h#111,112

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8870760 [details]
Bug 1365871 - stylo: Make editor/libeditor/tests pass by removing MOZ_CRASH from CSSEditUtils.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/142258/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/780eda199894
stylo: Make editor/libeditor/tests pass by removing MOZ_CRASH from CSSEditUtils. r=xidorn

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/780eda199894