Bug 178201

Summary:

Replace some stack raw pointers with RefPtrs within WebCore/html

Product:

WebKit

Reporter:

Jiewen Tan <jiewen_tan>

Component:

DOM

Assignee:

Jiewen Tan <jiewen_tan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, cdumez, commit-queue, jiewen_tan, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Archive of layout-test-results from ews115 for mac-elcapitan	none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews102 for mac-elcapitan	none
Patch	none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews115 for mac-elcapitan	none
Archive of layout-test-results from ews124 for ios-simulator-wk2	none
Patch	none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews102 for mac-elcapitan	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	rniwa: review+
Patch for landing	none

Jiewen Tan

Reported 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.

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
Show Obsolete (18) View All Add attachment proposed patch, testcase, etc.

Jiewen Tan

Comment 1 2017-10-11 19:37:36 PDT

<rdar://problem/34841692>

Jiewen Tan

Comment 2 2017-10-11 19:50:37 PDT

Created attachment 323510 [details] Patch

Jiewen Tan

Comment 3 2017-10-12 12:01:39 PDT

Created attachment 323541 [details] Patch

Build Bot

Comment 4 2017-10-12 12:04:28 PDT Comment hidden (obsolete)

Attachment 323541 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLMapElement.cpp:61: 'defaultArea' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLElement.cpp:882: 'oldMarkedNode' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/RadioNodeList.cpp:91: 'formElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLTableElement.cpp:235: 'row' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:221: 'beforeElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:770: 'foundSelected' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:771: 'firstOption' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:861: 'element' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1056: 'firstOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1057: 'selectedOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] 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/HTMLTableRowsCollection.cpp:78: 'child' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLFormControlElement.cpp:123: 'previousAncestor' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [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:175: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:187: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4981: 'texture' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] Total errors found: 17 in 78 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 5 2017-10-12 14:56:19 PDT Comment hidden (obsolete)

Comment on attachment 323541 [details] Patch Attachment 323541 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4836761 Number of test failures exceeded the failure limit.

Build Bot

Comment 6 2017-10-12 14:56:21 PDT Comment hidden (obsolete)

Created attachment 323572 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 7 2017-10-12 17:02:52 PDT Comment hidden (obsolete)

Comment on attachment 323541 [details] Patch Attachment 323541 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4838685 Number of test failures exceeded the failure limit.

Build Bot

Comment 8 2017-10-12 17:02:53 PDT Comment hidden (obsolete)

Created attachment 323604 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 9 2017-10-12 21:26:32 PDT Comment hidden (obsolete)

Comment on attachment 323541 [details] Patch Attachment 323541 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4841079 Number of test failures exceeded the failure limit.

Build Bot

Comment 10 2017-10-12 21:26:34 PDT Comment hidden (obsolete)

Created attachment 323628 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Jiewen Tan

Comment 11 2017-10-12 23:17:02 PDT

Created attachment 323642 [details] Patch

Build Bot

Comment 12 2017-10-12 23:19:30 PDT Comment hidden (obsolete)

Attachment 323642 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLMapElement.cpp:61: 'defaultArea' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLElement.cpp:882: 'oldMarkedNode' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/RadioNodeList.cpp:91: 'formElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLTableElement.cpp:235: 'row' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:221: 'beforeElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:770: 'foundSelected' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:771: 'firstOption' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:861: 'element' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1056: 'firstOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1057: 'selectedOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] 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/HTMLTableRowsCollection.cpp:78: 'child' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLFormControlElement.cpp:123: 'previousAncestor' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [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:110: 'newForm' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:175: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:187: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4981: 'texture' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] Total errors found: 18 in 78 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 13 2017-10-13 00:13:54 PDT Comment hidden (obsolete)

Comment on attachment 323642 [details] Patch Attachment 323642 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4842459 Number of test failures exceeded the failure limit.

Build Bot

Comment 14 2017-10-13 00:13:55 PDT Comment hidden (obsolete)

Created attachment 323644 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 15 2017-10-13 00:14:49 PDT Comment hidden (obsolete)

Comment on attachment 323642 [details] Patch Attachment 323642 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4842452 Number of test failures exceeded the failure limit.

Build Bot

Comment 16 2017-10-13 00:14:51 PDT Comment hidden (obsolete)

Created attachment 323645 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 17 2017-10-13 02:30:15 PDT Comment hidden (obsolete)

Comment on attachment 323642 [details] Patch Attachment 323642 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4843180 Number of test failures exceeded the failure limit.

Build Bot

Comment 18 2017-10-13 02:30:16 PDT Comment hidden (obsolete)

Created attachment 323652 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6

Ryosuke Niwa

Comment 19 2017-10-13 10:28:46 PDT

Looks like tests are failing?

Jiewen Tan

Comment 20 2017-10-13 11:35:00 PDT

Created attachment 323711 [details] Patch

Build Bot

Comment 21 2017-10-13 11:37:26 PDT Comment hidden (obsolete)

Attachment 323711 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLMapElement.cpp:61: 'defaultArea' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLElement.cpp:882: 'oldMarkedNode' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/RadioNodeList.cpp:91: 'formElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLTableElement.cpp:235: 'row' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:221: 'beforeElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:770: 'foundSelected' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:771: 'firstOption' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:861: 'element' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1056: 'firstOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1057: 'selectedOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] 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/HTMLTableRowsCollection.cpp:78: 'child' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLFormControlElement.cpp:123: 'previousAncestor' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [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:110: 'newForm' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:175: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:187: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4981: 'texture' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] Total errors found: 18 in 78 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jiewen Tan

Comment 22 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.

Jiewen Tan

Comment 23 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

Build Bot

Comment 24 2017-10-13 13:21:27 PDT Comment hidden (obsolete)

Comment on attachment 323711 [details] Patch Attachment 323711 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4849397 New failing tests: fast/forms/radio/radio-group-document-destruction.html

Build Bot

Comment 25 2017-10-13 13:21:29 PDT Comment hidden (obsolete)

Created attachment 323732 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 26 2017-10-13 13:21:35 PDT Comment hidden (obsolete)

Comment on attachment 323711 [details] Patch Attachment 323711 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4849490 New failing tests: fast/forms/radio/radio-group-document-destruction.html imported/w3c/web-platform-tests/html/semantics/forms/constraints/form-validation-reportValidity.html

Build Bot

Comment 27 2017-10-13 13:21:36 PDT Comment hidden (obsolete)

Created attachment 323733 [details] Archive of layout-test-results from ews102 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Jiewen Tan

Comment 28 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

Jiewen Tan

Comment 29 2017-10-13 13:29:25 PDT

Also the following places: HTMLFormControlElement::updateValidity:567, 570 if (HTMLFormElement* form = this->form());

Jiewen Tan

Comment 30 2017-10-13 13:31:44 PDT

Created attachment 323736 [details] Patch

Build Bot

Comment 31 2017-10-13 13:35:03 PDT Comment hidden (obsolete)

Attachment 323736 [details] did not pass style-queue: ERROR: Source/WebCore/html/HTMLMapElement.cpp:61: 'defaultArea' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLElement.cpp:882: 'oldMarkedNode' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/RadioNodeList.cpp:91: 'formElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLTableElement.cpp:235: 'row' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:221: 'beforeElement' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:770: 'foundSelected' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:771: 'firstOption' is incorrectly named. It should be named 'protector' or 'protected0'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:861: 'element' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1056: 'firstOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLSelectElement.cpp:1057: 'selectedOption' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] 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/HTMLTableRowsCollection.cpp:78: 'child' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/HTMLFormControlElement.cpp:123: 'previousAncestor' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [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:110: 'newForm' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:175: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/FormAssociatedElement.cpp:187: 'originalForm' is incorrectly named. It should be named 'protector' or 'protectedForm'. [readability/naming/protected] [4] ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:4981: 'texture' is incorrectly named. It should be named 'protector' or 'protectedNullptr'. [readability/naming/protected] [4] Total errors found: 18 in 78 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jiewen Tan

Comment 32 2017-10-14 02:10:24 PDT

No performance regressions has been detected on a local run of speedometer. Will initiate performance try-bots.

Ryosuke Niwa

Comment 33 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?

Jiewen Tan

Comment 34 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.

Jiewen Tan

Comment 35 2017-10-17 15:43:11 PDT

Created attachment 324065 [details] Patch

Build Bot

Comment 36 2017-10-17 15:46:16 PDT Comment hidden (obsolete)

Attachment 324065 [details] did not pass style-queue: 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 129 files If any of these errors are false positives, please file a bug against check-webkit-style.

Ryosuke Niwa

Comment 37 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(); ^

Jiewen Tan

Comment 38 2017-10-17 18:19:38 PDT

Created attachment 324076 [details] Patch

Build Bot

Comment 39 2017-10-17 18:22:02 PDT Comment hidden (obsolete)

Attachment 324076 [details] did not pass style-queue: 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 133 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jiewen Tan

Comment 40 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.

Jiewen Tan

Comment 41 2017-10-17 18:48:34 PDT

Created attachment 324080 [details] Patch

Build Bot

Comment 42 2017-10-17 18:50:29 PDT Comment hidden (obsolete)

Attachment 324080 [details] did not pass style-queue: 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 134 files If any of these errors are false positives, please file a bug against check-webkit-style.

Jiewen Tan

Comment 43 2017-10-17 18:59:08 PDT

Created attachment 324082 [details] Patch

Build Bot

Comment 44 2017-10-17 19:01:25 PDT Comment hidden (obsolete)

Attachment 324082 [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.

Jiewen Tan

Comment 45 2017-10-17 19:06:09 PDT

Created attachment 324084 [details] Patch

Build Bot

Comment 46 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.

Ryosuke Niwa

Comment 47 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.

Jiewen Tan

Comment 48 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.

Jiewen Tan

Comment 49 2017-10-18 13:08:08 PDT

Created attachment 324156 [details] Patch for landing

Build Bot

Comment 50 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.

Jiewen Tan

Comment 51 2017-10-18 18:01:27 PDT

Committed r223644: <https://trac.webkit.org/changeset/223644>

Jiewen Tan

Comment 52 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.

Note You need to log in before you can comment on or make changes to this bug.