Bug 179218 – Replace some auto* with RefPtr within WebCore/html

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Jiewen Tan]

Convert auto* to RefPtr within WebCore/html if appropriate.

[Jiewen Tan]

<rdar://problem/35102567>

[Jiewen Tan]

Created attachment 325804 [details]
Patch

[Build Bot]

Comment on attachment 325804 [details]
Patch

Attachment 325804 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5083053

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325812 [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

[Build Bot]

Comment on attachment 325804 [details]
Patch

Attachment 325804 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5083067

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325813 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 325804 [details]
Patch

Attachment 325804 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5083042

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325814 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 325804 [details]
Patch

Attachment 325804 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5083011

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325817 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Jiewen Tan]

Created attachment 325822 [details]
Patch

[Build Bot]

Comment on attachment 325822 [details]
Patch

Attachment 325822 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5084094

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325828 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 325822 [details]
Patch

Attachment 325822 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5084136

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325829 [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 on attachment 325822 [details]
Patch

Attachment 325822 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5084149

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325836 [details]
Archive of layout-test-results from ews116 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 325822 [details]
Patch

Attachment 325822 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5084208

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325837 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[Jiewen Tan]

Created attachment 325839 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 325839 [details]
Patch

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

> Source/WebCore/accessibility/AXObjectCache.cpp:1526
> +    RefPtr<HTMLElement> correspondingControl = downcast<HTMLLabelElement>(*element).control();

We can't use makeRefPtr & auto here?

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2527
> -    HTMLElement* correspondingControl = labelElement->control();
> +    RefPtr<HTMLElement> correspondingControl = labelElement->control();

Ditto.

[Ryosuke Niwa]

Comment on attachment 325839 [details]
Patch

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

> Source/WebCore/accessibility/AccessibilityTable.cpp:396
> +        if (RefPtr<HTMLTableCaptionElement> caption = tableElement->caption()) {

Use auto?

> Source/WebCore/accessibility/AccessibilityTable.cpp:690
> -        if (HTMLTableCaptionElement* caption = downcast<HTMLTableElement>(*tableElement).caption())
> +        if (RefPtr<HTMLTableCaptionElement> caption = downcast<HTMLTableElement>(*tableElement).caption())

Ditto.

> Source/WebCore/html/HTMLSummaryElement.cpp:76
> +    auto parent = makeRefPtr(parentElement());

It's a bit silly to use a RefPtr here since we're just downcast'ing to another pointer type.
I think it's okay to keep parent & host raw pointers as long as the return type is a RefPtr.

> Source/WebCore/html/HTMLTableRowElement.cpp:59
> -    auto* parent = row.parentNode();
> +    auto parent = makeRefPtr(row.parentNode());

Again, not certain if it's worth converting these local variables to RefPtr since they're just casted to another pointer type.

[Jiewen Tan]

Comment on attachment 325839 [details]
Patch

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

Thanks Ryosuke for r+ my patch. I will fix all crashes before landing the patch.

>> Source/WebCore/accessibility/AXObjectCache.cpp:1526
>> +    RefPtr<HTMLElement> correspondingControl = downcast<HTMLLabelElement>(*element).control();
> 
> We can't use makeRefPtr & auto here?

Fixed.

>> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2527
>> +    RefPtr<HTMLElement> correspondingControl = labelElement->control();
> 
> Ditto.

Fixed.

>> Source/WebCore/accessibility/AccessibilityTable.cpp:396
>> +        if (RefPtr<HTMLTableCaptionElement> caption = tableElement->caption()) {
> 
> Use auto?

Fixed.

>> Source/WebCore/accessibility/AccessibilityTable.cpp:690
>> +        if (RefPtr<HTMLTableCaptionElement> caption = downcast<HTMLTableElement>(*tableElement).caption())
> 
> Ditto.

Fixed.

>> Source/WebCore/html/HTMLSummaryElement.cpp:76
>> +    auto parent = makeRefPtr(parentElement());
> 
> It's a bit silly to use a RefPtr here since we're just downcast'ing to another pointer type.
> I think it's okay to keep parent & host raw pointers as long as the return type is a RefPtr.

Fixed.

>> Source/WebCore/html/HTMLTableRowElement.cpp:59
>> +    auto parent = makeRefPtr(row.parentNode());
> 
> Again, not certain if it's worth converting these local variables to RefPtr since they're just casted to another pointer type.

Fixed.

[Build Bot]

Comment on attachment 325839 [details]
Patch

Attachment 325839 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5085143

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325843 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 325839 [details]
Patch

Attachment 325839 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5085221

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325845 [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

[Build Bot]

Comment on attachment 325839 [details]
Patch

Attachment 325839 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5085201

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325848 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 325839 [details]
Patch

Attachment 325839 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/5085251

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 325851 [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

[Jiewen Tan]

Created attachment 325874 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 325874 [details]
Patch for landing

Clearing flags on attachment: 325874

Committed r224390: <https://trac.webkit.org/changeset/224390>

[zalan]

(In reply to WebKit Commit Bot from comment #33)
> Comment on attachment 325874 [details]
> Patch for landing
> 
> Clearing flags on attachment: 325874
> 
> Committed r224390: <https://trac.webkit.org/changeset/224390>
regression -> bug 179854