Bug 166710 – Crash inside Editor::styleForSelectionStart

• Component: HTML Editing
• Status: RESOLVED FIXED

[Ryosuke Niwa]

0   com.apple.WebCore             	0x00007fffdbc6b2c4 WebCore::checkAcceptChild(WebCore::ContainerNode&, WebCore::Node&, WebCore::Node const*, WebCore::Document::AcceptChildOperation) + 36
1   com.apple.WebCore             	0x00007fffdbc6ceb7 WebCore::ContainerNode::appendChild(WebCore::Node&) + 39
2   com.apple.WebCore             	0x00007fffdbe26014 WebCore::Editor::styleForSelectionStart(WebCore::Frame*, WebCore::Node*&) + 628
3   com.apple.WebKit              	0x00007fffdd15bf98 WebKit::WebPage::editorState(WebKit::WebPage::IncludePostLayoutDataHint) const + 368
4   com.apple.WebKit              	0x00007fffdd15c357 WebKit::WebPage::updateEditorStateAfterLayoutIfEditabilityChanged() + 91
5   com.apple.WebCore             	0x00007fffdbeece3d WebCore::FrameSelection::updateAppearanceAfterLayout() + 45
6   com.apple.WebCore             	0x00007fffdb96aac1 WebCore::FrameView::performPostLayoutTasks() + 65
7   com.apple.WebCore             	0x00007fffdb9609b1 WebCore::FrameView::layout(bool) + 3969
8   com.apple.WebCore             	0x00007fffdb9c7f4b WebCore::Document::updateLayout() + 187
9   com.apple.WebCore             	0x00007fffdbdb1047 WebCore::Document::updateLayoutIgnorePendingStylesheets(WebCore::Document::RunPostLayoutTasks) + 295
10  com.apple.WebCore             	0x00007fffdb9f818d WebCore::Element::offsetTop() + 29
11  com.apple.WebCore             	0x00007fffdc21bff8 WebCore::jsElementOffsetTop(JSC::ExecState*, long long, JSC::PropertyName) + 72
12  com.apple.JavaScriptCore      	0x00007fffd75eef90 JSC::getByVal(JSC::ExecState*, JSC::JSValue, JSC::JSValue, JSC::ByValInfo*, JSC::ReturnAddressPtr) + 5760

<rdar://problem/29763079>

[Ryosuke Niwa]

Created attachment 298060 [details]
Fixes the crash

[Darin Adler]

Comment on attachment 298060 [details]
Fixes the crash

View in context: https://bugs.webkit.org/attachment.cgi?id=298060&action=review

> Source/WebCore/editing/cocoa/EditorCocoa.mm:79
> +    RefPtr<Node> positionNode = position.deprecatedNode();

Why not Node* (or auto or auto*) instead of RefPtr<Node>? Doesn’t seem necessary to churn the reference count here.

[Ryosuke Niwa]

(In reply to comment #2)
> Comment on attachment 298060 [details]
> Fixes the crash
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=298060&action=review
> 
> > Source/WebCore/editing/cocoa/EditorCocoa.mm:79
> > +    RefPtr<Node> positionNode = position.deprecatedNode();
> 
> Why not Node* (or auto or auto*) instead of RefPtr<Node>? Doesn’t seem
> necessary to churn the reference count here.

I'd use auto here. I think using a raw pointer in editing code is a good idea because if someone starts using that raw pointe after calling appendChild, that could turn into a security vulnerability. And the risk and the cost of having to think about all those implications everywhere in editing is worth avoiding one ref churn.

[Ryosuke Niwa]

Committed r210376: <http://trac.webkit.org/changeset/210376>