Bug 178201 - Replace some stack raw pointers with RefPtrs within WebCore/html
Summary: Replace some stack raw pointers with RefPtrs within WebCore/html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-11 19:37 PDT by Jiewen Tan
Modified: 2017-10-18 18:05 PDT (History)
6 users (show)

See Also:


Attachments
Patch (195.73 KB, patch)
2017-10-11 19:50 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (186.46 KB, patch)
2017-10-12 12:01 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-elcapitan (1.53 MB, application/zip)
2017-10-12 14:56 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (399.67 KB, application/zip)
2017-10-12 17:02 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-elcapitan (397.87 KB, application/zip)
2017-10-12 21:26 PDT, Build Bot
no flags Details
Patch (187.20 KB, patch)
2017-10-12 23:17 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (750.90 KB, application/zip)
2017-10-13 00:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (603.35 KB, application/zip)
2017-10-13 00:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (761.61 KB, application/zip)
2017-10-13 02:30 PDT, Build Bot
no flags Details
Patch (186.77 KB, patch)
2017-10-13 11:35 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.89 MB, application/zip)
2017-10-13 13:21 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-elcapitan (1.31 MB, application/zip)
2017-10-13 13:21 PDT, Build Bot
no flags Details
Patch (185.48 KB, patch)
2017-10-13 13:31 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (242.76 KB, patch)
2017-10-17 15:43 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (245.68 KB, patch)
2017-10-17 18:19 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (246.91 KB, patch)
2017-10-17 18:48 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (247.71 KB, patch)
2017-10-17 18:59 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch (247.81 KB, patch)
2017-10-17 19:06 PDT, Jiewen Tan
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (248.02 KB, patch)
2017-10-18 13:08 PDT, Jiewen Tan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2017-10-11 19:37:01 PDT
Replace some stack raw pointers with RefPtrs within WebCore/html. This is an effort to reduce raw pointer usage in DOM code.
Comment 1 Jiewen Tan 2017-10-11 19:37:36 PDT
<rdar://problem/34841692>
Comment 2 Jiewen Tan 2017-10-11 19:50:37 PDT
Created attachment 323510 [details]
Patch
Comment 3 Jiewen Tan 2017-10-12 12:01:39 PDT
Created attachment 323541 [details]
Patch
Comment 4 Build Bot 2017-10-12 12:04:28 PDT Comment hidden (obsolete)
Comment 5 Build Bot 2017-10-12 14:56:19 PDT Comment hidden (obsolete)
Comment 6 Build Bot 2017-10-12 14:56:21 PDT Comment hidden (obsolete)
Comment 7 Build Bot 2017-10-12 17:02:52 PDT Comment hidden (obsolete)
Comment 8 Build Bot 2017-10-12 17:02:53 PDT Comment hidden (obsolete)
Comment 9 Build Bot 2017-10-12 21:26:32 PDT Comment hidden (obsolete)
Comment 10 Build Bot 2017-10-12 21:26:34 PDT Comment hidden (obsolete)
Comment 11 Jiewen Tan 2017-10-12 23:17:02 PDT
Created attachment 323642 [details]
Patch
Comment 12 Build Bot 2017-10-12 23:19:30 PDT Comment hidden (obsolete)
Comment 13 Build Bot 2017-10-13 00:13:54 PDT Comment hidden (obsolete)
Comment 14 Build Bot 2017-10-13 00:13:55 PDT Comment hidden (obsolete)
Comment 15 Build Bot 2017-10-13 00:14:49 PDT Comment hidden (obsolete)
Comment 16 Build Bot 2017-10-13 00:14:51 PDT Comment hidden (obsolete)
Comment 17 Build Bot 2017-10-13 02:30:15 PDT Comment hidden (obsolete)
Comment 18 Build Bot 2017-10-13 02:30:16 PDT Comment hidden (obsolete)
Comment 19 Ryosuke Niwa 2017-10-13 10:28:46 PDT
Looks like tests are failing?
Comment 20 Jiewen Tan 2017-10-13 11:35:00 PDT
Created attachment 323711 [details]
Patch
Comment 21 Build Bot 2017-10-13 11:37:26 PDT Comment hidden (obsolete)
Comment 22 Jiewen Tan 2017-10-13 11:42:07 PDT
First round inspection:
Many of the failing are because of hitting the following assertions:
Node.h:711: ASSERT(!m_deletionHasBegun);

This is due to the following change:
HTMLFormControlElement::willChangeForm:
Changing:
if (HTMLFormElement* form = this->form())
To:
if (RefPtr<HTMLFormElement> form = this->form())

Why this change violates the assertion?
Because we call HTMLFormControlElement::willChangeForm while destroying a HTMLFormElement. Therefore we hit the assertion as we are trying to ref the object again while derefing the object. This might actually be a bug. I will fire a bug later on to address this issue. For now, I will just revert the change and keep the raw pointer.
Comment 23 Jiewen Tan 2017-10-13 12:08:01 PDT
Here is the crash log:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x00000004a6cf8524 WTFCrash + 36 (Assertions.cpp:270)
1   com.apple.WebCore             	0x000000049801a821 WebCore::Node::ref() + 129 (Node.h:711)
2   com.apple.WebCore             	0x0000000498b0fa11 void WTF::refIfNotNull<WebCore::HTMLFormElement>(WebCore::HTMLFormElement*) + 49 (RefPtr.h:40)
3   com.apple.WebCore             	0x0000000498b0f9d4 WTF::RefPtr<WebCore::HTMLFormElement>::RefPtr(WebCore::HTMLFormElement*) + 36 (RefPtr.h:57)
4   com.apple.WebCore             	0x0000000498b0ec2d WTF::RefPtr<WebCore::HTMLFormElement>::RefPtr(WebCore::HTMLFormElement*) + 29 (RefPtr.h:57)
5   com.apple.WebCore             	0x0000000498dd0c65 WebCore::HTMLFormControlElement::willChangeForm() + 37 (HTMLFormControlElement.cpp:539)
6   com.apple.WebCore             	0x0000000498dfcc52 WebCore::HTMLInputElement::willChangeForm() + 34 (HTMLInputElement.cpp:1497)
7   com.apple.WebCore             	0x0000000498dfcc79 non-virtual thunk to WebCore::HTMLInputElement::willChangeForm() + 25
8   com.apple.WebCore             	0x0000000498b0f070 WebCore::FormAssociatedElement::formWillBeDestroyed() + 112 (FormAssociatedElement.cpp:169)
9   com.apple.WebCore             	0x0000000498dd971e WebCore::HTMLFormElement::~HTMLFormElement() + 238 (HTMLFormElement.cpp:83)
10  com.apple.WebCore             	0x0000000498dd99c5 WebCore::HTMLFormElement::~HTMLFormElement() + 21 (HTMLFormElement.cpp:87)
11  com.apple.WebCore             	0x0000000498dd99e9 WebCore::HTMLFormElement::~HTMLFormElement() + 25 (HTMLFormElement.cpp:77)
12  com.apple.WebCore             	0x000000049a229d2b WebCore::Node::removedLastRef() + 91 (Node.cpp:2541)
13  com.apple.WebCore             	0x000000049801ab43 WebCore::Node::deref() + 371 (Node.h:730)
14  com.apple.WebCore             	0x0000000498044cee void WTF::derefIfNotNull<WebCore::Node>(WebCore::Node*) + 46 (RefPtr.h:46)
15  com.apple.WebCore             	0x0000000498044cb3 WTF::RefPtr<WebCore::Node>::~RefPtr() + 83 (RefPtr.h:69)
16  com.apple.WebCore             	0x0000000498044c55 WTF::RefPtr<WebCore::Node>::~RefPtr() + 21 (RefPtr.h:69)
17  com.apple.WebCore             	0x0000000498045ab9 WTF::RefPtr<WebCore::Node>::operator=(WTF::RefPtr<WebCore::Node> const&) + 73 (RefPtr.h:133)
18  com.apple.WebCore             	0x00000004983edda3 WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&) + 627 (ContainerNodeAlgorithms.cpp:159)
19  com.apple.WebCore             	0x00000004983ede40 WebCore::removeDetachedChildrenInContainer(WebCore::ContainerNode&) + 48 (ContainerNodeAlgorithms.cpp:202)
20  com.apple.WebCore             	0x00000004983dac49 WebCore::ContainerNode::removeDetachedChildren() + 105 (ContainerNode.cpp:105)
21  com.apple.WebCore             	0x00000004983db75e WebCore::ContainerNode::~ContainerNode() + 94 (ContainerNode.cpp:161)
22  com.apple.WebCore             	0x000000049897bd1c WebCore::Element::~Element() + 380 (Element.cpp:195)
23  com.apple.WebCore             	0x000000049aab75a2 WebCore::StyledElement::~StyledElement() + 82 (StyledElement.cpp:62)
24  com.apple.WebCore             	0x00000004981b8635 WebCore::HTMLElement::~HTMLElement() + 21 (HTMLElement.h:38)
25  com.apple.WebCore             	0x0000000498d5adb5 WebCore::HTMLBodyElement::~HTMLBodyElement() + 21 (HTMLBodyElement.cpp:71)
26  com.apple.WebCore             	0x0000000498d5add5 WebCore::HTMLBodyElement::~HTMLBodyElement() + 21 (HTMLBodyElement.cpp:71)
27  com.apple.WebCore             	0x0000000498d5adf9 WebCore::HTMLBodyElement::~HTMLBodyElement() + 25 (HTMLBodyElement.cpp:70)
28  com.apple.WebCore             	0x000000049a229d2b WebCore::Node::removedLastRef() + 91 (Node.cpp:2541)
29  com.apple.WebCore             	0x000000049801ab43 WebCore::Node::deref() + 371 (Node.h:730)
30  com.apple.WebCore             	0x0000000498044cee void WTF::derefIfNotNull<WebCore::Node>(WebCore::Node*) + 46 (RefPtr.h:46)
31  com.apple.WebCore             	0x0000000498044cb3 WTF::RefPtr<WebCore::Node>::~RefPtr() + 83 (RefPtr.h:69)
32  com.apple.WebCore             	0x0000000498044c55 WTF::RefPtr<WebCore::Node>::~RefPtr() + 21 (RefPtr.h:69)
33  com.apple.WebCore             	0x0000000498045ab9 WTF::RefPtr<WebCore::Node>::operator=(WTF::RefPtr<WebCore::Node> const&) + 73 (RefPtr.h:133)
34  com.apple.WebCore             	0x00000004983edda3 WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&) + 627 (ContainerNodeAlgorithms.cpp:159)
35  com.apple.WebCore             	0x00000004983ede40 WebCore::removeDetachedChildrenInContainer(WebCore::ContainerNode&) + 48 (ContainerNodeAlgorithms.cpp:202)
36  com.apple.WebCore             	0x00000004983dac49 WebCore::ContainerNode::removeDetachedChildren() + 105 (ContainerNode.cpp:105)
37  com.apple.WebCore             	0x00000004983db75e WebCore::ContainerNode::~ContainerNode() + 94 (ContainerNode.cpp:161)
38  com.apple.WebCore             	0x000000049897bd1c WebCore::Element::~Element() + 380 (Element.cpp:195)
39  com.apple.WebCore             	0x000000049aab75a2 WebCore::StyledElement::~StyledElement() + 82 (StyledElement.cpp:62)
40  com.apple.WebCore             	0x00000004981b8635 WebCore::HTMLElement::~HTMLElement() + 21 (HTMLElement.h:38)
41  com.apple.WebCore             	0x0000000498de9a95 WebCore::HTMLHtmlElement::~HTMLHtmlElement() + 21 (HTMLHtmlElement.h:30)
42  com.apple.WebCore             	0x0000000498de9a45 WebCore::HTMLHtmlElement::~HTMLHtmlElement() + 21 (HTMLHtmlElement.h:30)
43  com.apple.WebCore             	0x0000000498de9a69 WebCore::HTMLHtmlElement::~HTMLHtmlElement() + 25 (HTMLHtmlElement.h:30)
44  com.apple.WebCore             	0x000000049a229d2b WebCore::Node::removedLastRef() + 91 (Node.cpp:2541)
45  com.apple.WebCore             	0x000000049801ab43 WebCore::Node::deref() + 371 (Node.h:730)
46  com.apple.WebCore             	0x0000000498044cee void WTF::derefIfNotNull<WebCore::Node>(WebCore::Node*) + 46 (RefPtr.h:46)
47  com.apple.WebCore             	0x0000000498044cb3 WTF::RefPtr<WebCore::Node>::~RefPtr() + 83 (RefPtr.h:69)
48  com.apple.WebCore             	0x0000000498044c55 WTF::RefPtr<WebCore::Node>::~RefPtr() + 21 (RefPtr.h:69)
49  com.apple.WebCore             	0x0000000498045ab9 WTF::RefPtr<WebCore::Node>::operator=(WTF::RefPtr<WebCore::Node> const&) + 73 (RefPtr.h:133)
50  com.apple.WebCore             	0x00000004983edda3 WebCore::addChildNodesToDeletionQueue(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode&) + 627 (ContainerNodeAlgorithms.cpp:159)
51  com.apple.WebCore             	0x00000004983ede40 WebCore::removeDetachedChildrenInContainer(WebCore::ContainerNode&) + 48 (ContainerNodeAlgorithms.cpp:202)
52  com.apple.WebCore             	0x00000004983dac49 WebCore::ContainerNode::removeDetachedChildren() + 105 (ContainerNode.cpp:105)
53  com.apple.WebCore             	0x000000049879e041 WebCore::Document::removedLastRef() + 529 (Document.cpp:652)
54  com.apple.WebCore             	0x000000049a229d07 WebCore::Node::removedLastRef() + 55 (Node.cpp:2534)
55  com.apple.WebCore             	0x000000049801ab43 WebCore::Node::deref() + 371 (Node.h:730)
56  com.apple.WebCore             	0x000000049a2204c5 WebCore::Node::derefEventTarget() + 21 (Node.cpp:756)
57  com.apple.WebCore             	0x00000004987de649 WebCore::EventTarget::deref() + 25 (EventTarget.h:66)
58  com.apple.WebCore             	0x000000049891184a WTF::Ref<WebCore::EventTarget>::~Ref() + 42 (Ref.h:59)
59  com.apple.WebCore             	0x00000004989015e5 WTF::Ref<WebCore::EventTarget>::~Ref() + 21 (Ref.h:59)
60  com.apple.WebCore             	0x0000000499603099 WebCore::JSDOMWrapper<WebCore::EventTarget>::~JSDOMWrapper() + 25 (JSDOMWrapper.h:79)
61  com.apple.WebCore             	0x0000000499603075 WebCore::JSEventTarget::~JSEventTarget() + 21 (JSEventTarget.h:30)
62  com.apple.WebCore             	0x0000000499601d15 WebCore::JSEventTarget::~JSEventTarget() + 21 (JSEventTarget.h:30)
63  com.apple.WebCore             	0x000000049960140d WebCore::JSEventTarget::destroy(JSC::JSCell*) + 29 (JSEventTarget.cpp:203)
64  com.apple.JavaScriptCore      	0x00000004a68bc80a JSC::(anonymous namespace)::DestroyFunc::operator()(JSC::VM&, JSC::JSCell*) const + 42 (JSDestructibleObjectSubspace.cpp:41)
65  com.apple.JavaScriptCore      	0x00000004a68f9525 void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::(anonymous namespace)::DestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::(anonymous namespace)::DestroyFunc const&)::'lambda'(void*)::operator()(void*) const + 69 (MarkedBlockInlines.h:169)
66  com.apple.JavaScriptCore      	0x00000004a68f9594 void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::(anonymous namespace)::DestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::(anonymous namespace)::DestroyFunc const&)::'lambda'(unsigned long)::operator()(unsigned long) const + 84 (MarkedBlockInlines.h:229)
67  com.apple.JavaScriptCore      	0x00000004a68f4525 void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::(anonymous namespace)::DestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::(anonymous namespace)::DestroyFunc const&) + 1269 (MarkedBlockInlines.h:250)
68  com.apple.JavaScriptCore      	0x00000004a68bc7a0 void JSC::MarkedBlock::Handle::finishSweepKnowingSubspace<JSC::(anonymous namespace)::DestroyFunc>(JSC::FreeList*, JSC::(anonymous namespace)::DestroyFunc const&) + 304 (MarkedBlockInlines.h:345)
69  com.apple.JavaScriptCore      	0x00000004a68bc668 JSC::JSDestructibleObjectSubspace::finishSweep(JSC::MarkedBlock::Handle&, JSC::FreeList*) + 40 (JSDestructibleObjectSubspace.cpp:58)
70  com.apple.JavaScriptCore      	0x00000004a64644a1 JSC::MarkedBlock::Handle::sweep(JSC::FreeList*) + 641 (MarkedBlock.cpp:429)
71  com.apple.JavaScriptCore      	0x00000004a6463ffd JSC::MarkedAllocator::tryAllocateIn(JSC::MarkedBlock::Handle*) + 157 (MarkedAllocator.cpp:142)
72  com.apple.JavaScriptCore      	0x00000004a6463c11 JSC::MarkedAllocator::tryAllocateWithoutCollecting() + 353 (MarkedAllocator.cpp:103)
73  com.apple.JavaScriptCore      	0x00000004a6464d4b JSC::MarkedAllocator::allocateSlowCaseImpl(JSC::GCDeferralContext*, bool) + 411 (MarkedAllocator.cpp:213)
74  com.apple.JavaScriptCore      	0x00000004a6464ba9 JSC::MarkedAllocator::allocateSlowCase(JSC::GCDeferralContext*) + 41 (MarkedAllocator.cpp:181)
75  com.apple.JavaScriptCore      	0x00000004a6470207 JSC::MarkedAllocator::allocate(JSC::GCDeferralContext*)::'lambda'()::operator()() const + 71 (MarkedAllocatorInlines.h:52)
76  com.apple.JavaScriptCore      	0x00000004a6470189 JSC::HeapCell* JSC::FreeList::allocate<JSC::MarkedAllocator::allocate(JSC::GCDeferralContext*)::'lambda'()>(JSC::MarkedAllocator::allocate(JSC::GCDeferralContext*)::'lambda'() const&) + 169 (FreeListInlines.h:46)
77  com.apple.JavaScriptCore      	0x00000004a6464fef JSC::MarkedAllocator::allocate(JSC::GCDeferralContext*) + 47 (MarkedAllocatorInlines.h:49)
78  com.apple.JavaScriptCore      	0x00000004a648617b JSC::Subspace::allocate(unsigned long) + 75 (Subspace.cpp:100)
79  com.apple.WebCore             	0x000000049940dda0 void* JSC::tryAllocateCellHelper<WebCore::JSHTMLDocument, (JSC::AllocationFailureMode)0, (JSC::GCDeferralContextArgPresense)1>(JSC::Heap&, JSC::GCDeferralContext*, unsigned long) + 176 (JSCellInlines.h:152)
80  com.apple.WebCore             	0x000000049940dce4 void* JSC::allocateCell<WebCore::JSHTMLDocument>(JSC::Heap&, unsigned long) + 36 (JSCellInlines.h:173)
81  com.apple.WebCore             	0x000000049940d9a3 WebCore::JSHTMLDocument::create(JSC::Structure*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::HTMLDocument>&&) + 51 (JSHTMLDocument.h:34)
82  com.apple.WebCore             	0x000000049940d862 std::__1::enable_if<std::is_same<WebCore::HTMLDocument, WebCore::HTMLDocument>::value, WebCore::JSDOMWrapperConverterTraits<WebCore::HTMLDocument>::WrapperClass*>::type WebCore::createWrapper<WebCore::HTMLDocument, WebCore::HTMLDocument>(WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::HTMLDocument>&&) + 162 (JSDOMWrapperCache.h:188)
83  com.apple.WebCore             	0x000000049940d66d std::__1::enable_if<!(std::is_same<WebCore::HTMLDocument, WebCore::Document>::value), WebCore::JSDOMWrapperConverterTraits<WebCore::HTMLDocument>::WrapperClass*>::type WebCore::createWrapper<WebCore::HTMLDocument, WebCore::Document>(WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Document>&&) + 61 (JSDOMWrapperCache.h:195)
84  com.apple.WebCore             	0x000000049940d497 WebCore::createNewDocumentWrapper(JSC::ExecState&, WebCore::JSDOMGlobalObject&, WTF::Ref<WebCore::Document>&&) + 87 (JSDocumentCustom.cpp:39)
85  com.apple.WebCore             	0x000000049940d430 WebCore::toJSNewlyCreated(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Document>&&) + 64 (JSDocumentCustom.cpp:81)
86  com.apple.WebCore             	0x000000049940d5aa WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Document&) + 122 (JSDocumentCustom.cpp:88)
87  com.apple.WebCore             	0x00000004999345b8 WebCore::createWrapperInline(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Node>&&) + 680 (JSNodeCustom.cpp:155)
88  com.apple.WebCore             	0x0000000499934300 WebCore::createWrapper(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WTF::Ref<WebCore::Node>&&) + 64 (JSNodeCustom.cpp:174)
89  com.apple.WebCore             	0x0000000498e3d4ae WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Node&) + 206 (JSNodeCustom.h:46)
90  com.apple.WebCore             	0x0000000498e34980 WebCore::toJS(JSC::ExecState*, WebCore::JSDOMGlobalObject*, WebCore::Node*) + 48 (JSNode.h:96)
91  com.apple.WebCore             	0x00000004995a923f WebCore::JSDOMWindowBase::updateDocument() + 239 (JSDOMWindowBase.cpp:122)
92  com.apple.WebCore             	0x000000049a8154b6 WebCore::ScriptController::updateDocument() + 166 (ScriptController.cpp:519)
93  com.apple.WebCore             	0x00000004987aa60b WebCore::Document::didBecomeCurrentDocumentInFrame() + 43 (Document.cpp:2182)
94  com.apple.WebCore             	0x0000000498baddf9 WebCore::Frame::setDocument(WTF::RefPtr<WebCore::Document>&&) + 649 (Frame.cpp:299)
95  com.apple.WebCore             	0x000000049887afd2 WebCore::DocumentWriter::begin(WebCore::URL const&, bool, WebCore::Document*) + 754 (DocumentWriter.cpp:174)
96  com.apple.WebCore             	0x000000049883098e WebCore::DocumentLoader::commitData(char const*, unsigned long) + 142 (DocumentLoader.cpp:851)
97  com.apple.WebKit              	0x0000000103aef99f WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 79 (WebFrameLoaderClient.cpp:999)
98  com.apple.WebCore             	0x0000000498833d2d WebCore::DocumentLoader::commitLoad(char const*, int) + 205 (DocumentLoader.cpp:831)
99  com.apple.WebCore             	0x0000000498833c4f WebCore::DocumentLoader::dataReceived(char const*, int) + 511 (DocumentLoader.cpp:945)
100 com.apple.WebCore             	0x0000000498834384 WebCore::DocumentLoader::dataReceived(WebCore::CachedResource&, char const*, int) + 116 (DocumentLoader.cpp:918)
101 com.apple.WebCore             	0x00000004988343ca non-virtual thunk to WebCore::DocumentLoader::dataReceived(WebCore::CachedResource&, char const*, int) + 58
102 com.apple.WebCore             	0x000000049828a708 WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int) + 152 (CachedRawResource.cpp:114)
103 com.apple.WebCore             	0x000000049828a56d WebCore::CachedRawResource::updateBuffer(WebCore::SharedBuffer&) + 253 (CachedRawResource.cpp:65)
104 com.apple.WebCore             	0x000000049ab3eeca WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::RefPtr<WebCore::SharedBuffer>&&, long long, WebCore::DataPayloadType) + 538 (SubresourceLoader.cpp:406)
105 com.apple.WebCore             	0x000000049ab3ec92 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType) + 98 (SubresourceLoader.cpp:374)
106 com.apple.WebKit              	0x0000000103f00684 WebKit::WebResourceLoader::didReceiveData(IPC::DataReference const&, long long) + 500 (WebResourceLoader.cpp:135)
107 com.apple.WebKit              	0x0000000103f04060 void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) + 192 (HandleMessage.h:41)
108 com.apple.WebKit              	0x0000000103f03e30 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<IPC::DataReference, long long>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 96 (HandleMessage.h:47)
109 com.apple.WebKit              	0x0000000103f03281 void IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 289 (HandleMessage.h:127)
110 com.apple.WebKit              	0x0000000103f02a46 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&) + 502 (WebResourceLoaderMessageReceiver.cpp:62)
111 com.apple.WebKit              	0x00000001035dbeb9 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 169 (NetworkProcessConnection.cpp:70)
112 com.apple.WebKit              	0x0000000103369143 IPC::Connection::dispatchMessage(IPC::Decoder&) + 51 (Connection.cpp:902)
113 com.apple.WebKit              	0x000000010335e728 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) + 712
114 com.apple.WebKit              	0x000000010336974a IPC::Connection::dispatchOneMessage() + 1530 (Connection.cpp:959)
115 com.apple.WebKit              	0x0000000103381a2d IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() + 29 (Connection.cpp:896)
116 com.apple.WebKit              	0x0000000103381989 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() + 25 (Function.h:101)
117 com.apple.JavaScriptCore      	0x00000004a6d2de0b WTF::Function<void ()>::operator()() const + 139 (Function.h:56)
118 com.apple.JavaScriptCore      	0x00000004a6d4e443 WTF::RunLoop::performWork() + 211 (RunLoop.cpp:107)
119 com.apple.JavaScriptCore      	0x00000004a6d4ecc4 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38)
120 com.apple.CoreFoundation      	0x00007fff473446b1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
121 com.apple.CoreFoundation      	0x00007fff473fe35c __CFRunLoopDoSource0 + 108
122 com.apple.CoreFoundation      	0x00007fff47327150 __CFRunLoopDoSources0 + 208
123 com.apple.CoreFoundation      	0x00007fff473265cd __CFRunLoopRun + 1293
124 com.apple.CoreFoundation      	0x00007fff47325e33 CFRunLoopRunSpecific + 483
125 com.apple.HIToolbox           	0x00007fff46645866 RunCurrentEventLoopInMode + 286
126 com.apple.HIToolbox           	0x00007fff466455d6 ReceiveNextEventCommon + 613
127 com.apple.HIToolbox           	0x00007fff46645354 _BlockUntilNextEventMatchingListInModeWithFilter + 64
128 com.apple.AppKit              	0x00007fff4494444f _DPSNextEvent + 2085
129 com.apple.AppKit              	0x00007fff450d9508 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 3044
130 com.apple.AppKit              	0x00007fff4493925d -[NSApplication run] + 764
131 com.apple.AppKit              	0x00007fff449083fe NSApplicationMain + 804
132 libxpc.dylib                  	0x00007fff6ed536c3 _xpc_objc_main + 580
133 libxpc.dylib                  	0x00007fff6ed52316 xpc_main + 417
134 com.apple.WebKit.WebContent   	0x000000010322413b main + 1195 (XPCServiceMain.mm:148)
135 libdyld.dylib                 	0x00007fff6ea87145 start + 1
Comment 24 Build Bot 2017-10-13 13:21:27 PDT Comment hidden (obsolete)
Comment 25 Build Bot 2017-10-13 13:21:29 PDT Comment hidden (obsolete)
Comment 26 Build Bot 2017-10-13 13:21:35 PDT Comment hidden (obsolete)
Comment 27 Build Bot 2017-10-13 13:21:36 PDT Comment hidden (obsolete)
Comment 28 Jiewen Tan 2017-10-13 13:22:59 PDT
Second round inspection:
There is another place identified could hit the same assertion as well.

HTMLFormControlElement::computeWillValidate
From:
for (ContainerNode* ancestor = parentNode(); ancestor; ancestor = ancestor->parentNode()
To:
for (RefPtr<ContainerNode> ancestor = parentNode(); ancestor; ancestor = ancestor->parentNode()

Same explanation as before, this one is just hidden behind the last crashing point.

Here is the crash log:
ASSERTION FAILED: !m_deletionHasBegun
/Users/jwtan/Documents/Source/OpenSource/Source/WebCore/dom/Node.h(711) : void WebCore::Node::ref()
1   0x776cf851d WTFCrash
2   0x76801a841 WebCore::Node::ref()
3   0x768124ea1 void WTF::refIfNotNull<WebCore::ContainerNode>(WebCore::ContainerNode*)
4   0x768124e64 WTF::RefPtr<WebCore::ContainerNode>::RefPtr(WebCore::ContainerNode*)
5   0x76811d3cd WTF::RefPtr<WebCore::ContainerNode>::RefPtr(WebCore::ContainerNode*)
6   0x768dcff07 WebCore::HTMLFormControlElement::computeWillValidate() const
7   0x768dfcfe6 WebCore::HTMLInputElement::computeWillValidate() const
8   0x768dcf494 WebCore::HTMLFormControlElement::willValidate() const
9   0x768dcec79 WebCore::HTMLFormControlElement::updateValidity()
10  0x76a38578e WebCore::RadioButtonGroup::remove(WebCore::HTMLInputElement*)
11  0x76a3870cf WebCore::RadioButtonGroups::removeButton(WebCore::HTMLInputElement*)
12  0x768df7485 WebCore::HTMLInputElement::removeFromRadioButtonGroup()
13  0x768dfcc49 WebCore::HTMLInputElement::willChangeForm()
14  0x768dfcc79 non-virtual thunk to WebCore::HTMLInputElement::willChangeForm()
15  0x768b0f090 WebCore::FormAssociatedElement::formWillBeDestroyed()
16  0x768dd971e WebCore::HTMLFormElement::~HTMLFormElement()
17  0x768dd99c5 WebCore::HTMLFormElement::~HTMLFormElement()
18  0x768dd99e9 WebCore::HTMLFormElement::~HTMLFormElement()
19  0x76a229d2b WebCore::Node::removedLastRef()
20  0x76801ab63 WebCore::Node::deref()
21  0x76a2204c5 WebCore::Node::derefEventTarget()
22  0x7687de669 WebCore::EventTarget::deref()
23  0x76891186a WTF::Ref<WebCore::EventTarget>::~Ref()
24  0x768901605 WTF::Ref<WebCore::EventTarget>::~Ref()
25  0x769603099 WebCore::JSDOMWrapper<WebCore::EventTarget>::~JSDOMWrapper()
26  0x769603075 WebCore::JSEventTarget::~JSEventTarget()
27  0x769601d15 WebCore::JSEventTarget::~JSEventTarget()
28  0x76960140d WebCore::JSEventTarget::destroy(JSC::JSCell*)
29  0x7768bc80a JSC::(anonymous namespace)::DestroyFunc::operator()(JSC::VM&, JSC::JSCell*) const
30  0x7768f9525 void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::(anonymous namespace)::DestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::(anonymous namespace)::DestroyFunc const&)::'lambda'(void*)::operator()(void*) const
31  0x7768f9594 void JSC::MarkedBlock::Handle::specializedSweep<false, (JSC::MarkedBlock::Handle::EmptyMode)0, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::MarkedBlock::Handle::SweepDestructionMode)0, (JSC::MarkedBlock::Handle::ScribbleMode)0, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)0, (JSC::MarkedBlock::Handle::MarksMode)0, JSC::(anonymous namespace)::DestroyFunc>(JSC::FreeList*, JSC::MarkedBlock::Handle::EmptyMode, JSC::MarkedBlock::Handle::SweepMode, JSC::MarkedBlock::Handle::SweepDestructionMode, JSC::MarkedBlock::Handle::ScribbleMode, JSC::MarkedBlock::Handle::NewlyAllocatedMode, JSC::MarkedBlock::Handle::MarksMode, JSC::(anonymous namespace)::DestroyFunc const&)::'lambda'(unsigned long)::operator()(unsigned long) const
LEAK: 1 WebPageProxy
Comment 29 Jiewen Tan 2017-10-13 13:29:25 PDT
Also the following places:
HTMLFormControlElement::updateValidity:567, 570
if (HTMLFormElement* form = this->form());
Comment 30 Jiewen Tan 2017-10-13 13:31:44 PDT
Created attachment 323736 [details]
Patch
Comment 31 Build Bot 2017-10-13 13:35:03 PDT Comment hidden (obsolete)
Comment 32 Jiewen Tan 2017-10-14 02:10:24 PDT
No performance regressions has been detected on a local run of speedometer. Will initiate performance try-bots.
Comment 33 Ryosuke Niwa 2017-10-14 18:01:53 PDT
Comment on attachment 323736 [details]
Patch

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

> Source/WebCore/html/FormAssociatedElement.cpp:113
> +            newForm = downcast<HTMLFormElement>(newFormCandidate.get());

Why don't we just return this?

> Source/WebCore/html/FormAssociatedElement.cpp:114
> +        return newForm.get();

And return nullptr here instead. That way, we can just get rid of newForm.

> Source/WebCore/html/HTMLFormControlElement.cpp:451
> -        if (HTMLFormElement* form = this->form())
> +        if (RefPtr<HTMLFormElement> form = this->form())

We should make form() return a RefPtr<HTMLFormElement> and just use auto here.

> Source/WebCore/html/HTMLFormElement.cpp:143
> -    Node* targetNode = event.target()->toNode();
> +    RefPtr<Node> targetNode = event.target()->toNode();

Can we make toNode return RefPtr<Node> instead?

> Source/WebCore/html/HTMLFormElement.cpp:272
> -    HTMLFormControlElement* submitElement = submitElementFromEvent(event);
> +    RefPtr<HTMLFormControlElement> submitElement = submitElementFromEvent(event);

Can we make submitElementFromEvent return RefPtr?

> Source/WebCore/html/HTMLFormElement.cpp:787
> -    HTMLElement* elementFromPast = elementFromPastNamesMap(name);
> +    RefPtr<HTMLElement> elementFromPast = elementFromPastNamesMap(name);

Make elementFromPastNamesMap return a RefPtr?

> Source/WebCore/html/HTMLFrameElement.cpp:67
> -    const HTMLFrameSetElement* containingFrameSet = HTMLFrameSetElement::findContaining(this);
> +    const RefPtr<HTMLFrameSetElement> containingFrameSet = HTMLFrameSetElement::findContaining(this);

Why don't we make findContaining return a RefPtr?

> Source/WebCore/html/HTMLFrameSetElement.cpp:179
> -    const HTMLFrameSetElement* containingFrameSet = findContaining(this);
> +    const RefPtr<HTMLFrameSetElement> containingFrameSet = findContaining(this);

Can we make findContaining return a RefPtr?

> Source/WebCore/html/HTMLImageElement.cpp:622
> -    ShadowRoot* shadowRoot = userAgentShadowRoot();
> +    RefPtr<ShadowRoot> shadowRoot = userAgentShadowRoot();

Make userAgentShadowRoot return a RefPtr?

> Source/WebCore/html/HTMLSelectElement.cpp:771
> +    RefPtr<HTMLOptionElement> foundSelected = 0;
> +    RefPtr<HTMLOptionElement> firstOption = 0;

No need to initialize them to 0. (Use nullptr if any).

> Source/WebCore/html/HTMLSelectElement.cpp:1057
> +    RefPtr<HTMLOptionElement> firstOption = nullptr;
> +    RefPtr<HTMLOptionElement> selectedOption = nullptr;

Get rid of initialization?

> Source/WebCore/html/HTMLTextFormControlElement.cpp:445
> -    TextControlInnerTextElement* innerText = innerTextElement();
> +    RefPtr<TextControlInnerTextElement> innerText = innerTextElement();

Can we make innerTextElement return a RefPtr?

> Source/WebCore/html/HTMLTrackElement.cpp:212
> -    HTMLMediaElement* parent = mediaElement();
> +    RefPtr<HTMLMediaElement> parent = mediaElement();

Can we make mediaElement() return a RefPtr?

> Source/WebCore/html/MediaElementSession.cpp:464
> -    MediaPlayer* player = element.player();
> +    RefPtr<MediaPlayer> player = element.player();

Can we make player return a RefPtr?

> Source/WebCore/html/shadow/MediaControlElements.cpp:736
> -    HTMLMediaElement* mediaElement = parentMediaElement(this);
> +    RefPtr<HTMLMediaElement> mediaElement = parentMediaElement(this);

Make parentMediaElement return RefPtr?

> Source/WebCore/html/shadow/SliderThumbElement.cpp:229
> -    HTMLInputElement* input = hostInput();
> +    RefPtr<HTMLInputElement> input = hostInput();

Make hostInput return a RefPtr?
Comment 34 Jiewen Tan 2017-10-17 14:31:08 PDT
Comment on attachment 323736 [details]
Patch

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

Thanks Ryosuke for r+ my patch. However, in order to address the comments, I made quite some changes. Therefore, the next patch will probably need reviews as well.

> Source/WebCore/ChangeLog:417
> +2017-10-06  Jiewen Tan  <jiewen_tan@apple.com>

Removed.

>> Source/WebCore/html/FormAssociatedElement.cpp:113
>> +            newForm = downcast<HTMLFormElement>(newFormCandidate.get());
> 
> Why don't we just return this?

Good catch.

>> Source/WebCore/html/FormAssociatedElement.cpp:114
>> +        return newForm.get();
> 
> And return nullptr here instead. That way, we can just get rid of newForm.

Done.

>> Source/WebCore/html/HTMLFormControlElement.cpp:451
>> +        if (RefPtr<HTMLFormElement> form = this->form())
> 
> We should make form() return a RefPtr<HTMLFormElement> and just use auto here.

It turns out that "this->form()" is used in cases of previous patches that we hit the assertion. Hence, I cannot address it in this patch.

>> Source/WebCore/html/HTMLFormElement.cpp:143
>> +    RefPtr<Node> targetNode = event.target()->toNode();
> 
> Can we make toNode return RefPtr<Node> instead?

Fixed.

>> Source/WebCore/html/HTMLFormElement.cpp:272
>> +    RefPtr<HTMLFormControlElement> submitElement = submitElementFromEvent(event);
> 
> Can we make submitElementFromEvent return RefPtr?

Fixed.

>> Source/WebCore/html/HTMLFormElement.cpp:787
>> +    RefPtr<HTMLElement> elementFromPast = elementFromPastNamesMap(name);
> 
> Make elementFromPastNamesMap return a RefPtr?

Fixed.

>> Source/WebCore/html/HTMLFrameElement.cpp:67
>> +    const RefPtr<HTMLFrameSetElement> containingFrameSet = HTMLFrameSetElement::findContaining(this);
> 
> Why don't we make findContaining return a RefPtr?

Fixed.

>> Source/WebCore/html/HTMLFrameSetElement.cpp:179
>> +    const RefPtr<HTMLFrameSetElement> containingFrameSet = findContaining(this);
> 
> Can we make findContaining return a RefPtr?

Fixed.

>> Source/WebCore/html/HTMLImageElement.cpp:622
>> +    RefPtr<ShadowRoot> shadowRoot = userAgentShadowRoot();
> 
> Make userAgentShadowRoot return a RefPtr?

Fixed.

>> Source/WebCore/html/HTMLSelectElement.cpp:771
>> +    RefPtr<HTMLOptionElement> firstOption = 0;
> 
> No need to initialize them to 0. (Use nullptr if any).

Fixed.

>> Source/WebCore/html/HTMLSelectElement.cpp:1057
>> +    RefPtr<HTMLOptionElement> selectedOption = nullptr;
> 
> Get rid of initialization?

Fixed.

>> Source/WebCore/html/HTMLTextFormControlElement.cpp:445
>> +    RefPtr<TextControlInnerTextElement> innerText = innerTextElement();
> 
> Can we make innerTextElement return a RefPtr?

Fixed.

>> Source/WebCore/html/HTMLTrackElement.cpp:212
>> +    RefPtr<HTMLMediaElement> parent = mediaElement();
> 
> Can we make mediaElement() return a RefPtr?

Fixed.

>> Source/WebCore/html/MediaElementSession.cpp:464
>> +    RefPtr<MediaPlayer> player = element.player();
> 
> Can we make player return a RefPtr?

Fixed.

>> Source/WebCore/html/shadow/MediaControlElements.cpp:736
>> +    RefPtr<HTMLMediaElement> mediaElement = parentMediaElement(this);
> 
> Make parentMediaElement return RefPtr?

Fixed.

>> Source/WebCore/html/shadow/SliderThumbElement.cpp:229
>> +    RefPtr<HTMLInputElement> input = hostInput();
> 
> Make hostInput return a RefPtr?

Fixed.
Comment 35 Jiewen Tan 2017-10-17 15:43:11 PDT
Created attachment 324065 [details]
Patch
Comment 36 Build Bot 2017-10-17 15:46:16 PDT Comment hidden (obsolete)
Comment 37 Ryosuke Niwa 2017-10-17 16:07:00 PDT
From WPE EWS bot:

/home/ews/igalia-wpe-ews/WebKit/Source/WebKit/WebProcess/WebCoreSupport/wpe/WebEditorClientWPE.cpp:211:42: error: cannot convert ‘WTF::RefPtr<WebCore::Node>’ to ‘WebCore::Node*’ in initialization
     Node* node = event->target()->toNode();
                                          ^
Comment 38 Jiewen Tan 2017-10-17 18:19:38 PDT
Created attachment 324076 [details]
Patch
Comment 39 Build Bot 2017-10-17 18:22:02 PDT Comment hidden (obsolete)
Comment 40 Jiewen Tan 2017-10-17 18:34:28 PDT
No performance regression has been detected in a local run of speedometer. Uploading this patch to performance try-bots.
Comment 41 Jiewen Tan 2017-10-17 18:48:34 PDT
Created attachment 324080 [details]
Patch
Comment 42 Build Bot 2017-10-17 18:50:29 PDT Comment hidden (obsolete)
Comment 43 Jiewen Tan 2017-10-17 18:59:08 PDT
Created attachment 324082 [details]
Patch
Comment 44 Build Bot 2017-10-17 19:01:25 PDT Comment hidden (obsolete)
Comment 45 Jiewen Tan 2017-10-17 19:06:09 PDT
Created attachment 324084 [details]
Patch
Comment 46 Build Bot 2017-10-17 19:07:57 PDT
Attachment 324084 [details] did not pass style-queue:


WARNING: File exempt from style guide. Skipping: "Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMPrivate.cpp"
ERROR: Source/WebCore/html/HTMLTextFormControlElement.cpp:598:  'lastBrOrText' is incorrectly named. It should be named 'protector' or 'protectedInnerText'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/html/HTMLFormElement.cpp:675:  'oldDefault' is incorrectly named. It should be named 'protector' or 'protectedDefaultButton'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/html/FormAssociatedElement.cpp:174:  'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/html/FormAssociatedElement.cpp:186:  'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'.  [readability/naming/protected] [4]
Total errors found: 4 in 135 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 47 Ryosuke Niwa 2017-10-17 22:59:24 PDT
Comment on attachment 324084 [details]
Patch

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

> Source/WebCore/html/InputType.h:219
> -    virtual TextControlInnerTextElement* innerTextElement() const { return nullptr; }
> +    virtual RefPtr<TextControlInnerTextElement> innerTextElement() const { return nullptr; }

Please move this implementation to the InputType.cpp instead of including TextControlInnerElements.h since this is a virtual function.
Comment 48 Jiewen Tan 2017-10-18 12:35:33 PDT
Comment on attachment 324084 [details]
Patch

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

Thanks Ryosuke for r+ my patch.

>> Source/WebCore/html/InputType.h:219
>> +    virtual RefPtr<TextControlInnerTextElement> innerTextElement() const { return nullptr; }
> 
> Please move this implementation to the InputType.cpp instead of including TextControlInnerElements.h since this is a virtual function.

Fixed.
Comment 49 Jiewen Tan 2017-10-18 13:08:08 PDT
Created attachment 324156 [details]
Patch for landing
Comment 50 Build Bot 2017-10-18 13:10:18 PDT
Attachment 324156 [details] did not pass style-queue:


WARNING: File exempt from style guide. Skipping: "Source/WebKit/WebProcess/InjectedBundle/API/gtk/DOM/WebKitDOMPrivate.cpp"
ERROR: Source/WebCore/html/HTMLTextFormControlElement.cpp:598:  'lastBrOrText' is incorrectly named. It should be named 'protector' or 'protectedInnerText'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/html/HTMLFormElement.cpp:675:  'oldDefault' is incorrectly named. It should be named 'protector' or 'protectedDefaultButton'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/html/FormAssociatedElement.cpp:174:  'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'.  [readability/naming/protected] [4]
ERROR: Source/WebCore/html/FormAssociatedElement.cpp:186:  'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'.  [readability/naming/protected] [4]
Total errors found: 4 in 135 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 51 Jiewen Tan 2017-10-18 18:01:27 PDT
Committed r223644: <https://trac.webkit.org/changeset/223644>
Comment 52 Jiewen Tan 2017-10-18 18:01:51 PDT
(In reply to Jiewen Tan from comment #51)
> Committed r223644: <https://trac.webkit.org/changeset/223644>

With conflicts resolved.