Bug 166569 – Remove PassRefPtr use from the "dom" directory, related cleanup

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Darin Adler]

Remove PassRefPtr use from the "dom" directory, related cleanup

[Darin Adler]

Created attachment 297822 [details]
Patch

[Darin Adler]

Created attachment 297824 [details]
Patch

[WebKit Commit Bot]

Attachment 297824 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:84:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:34:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 5 in 70 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 297824 [details]
Patch

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

New failing tests:
http/tests/globalhistory/history-delegate-basic-title.html
fast/dom/title-text-property-2.html

[Build Bot]

Created attachment 297827 [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 297824 [details]
Patch

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

New failing tests:
http/tests/globalhistory/history-delegate-basic-title.html
fast/dom/title-text-property-2.html

[Build Bot]

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

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

[Build Bot]

Comment on attachment 297824 [details]
Patch

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

New failing tests:
http/tests/globalhistory/history-delegate-basic-title.html
fast/dom/title-text-property-2.html

[Build Bot]

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

[Darin Adler]

Created attachment 297830 [details]
Patch

[WebKit Commit Bot]

Attachment 297830 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MouseEvent.cpp:84:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:34:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 5 in 72 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Created attachment 297849 [details]
Patch

[WebKit Commit Bot]

Attachment 297849 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MouseEvent.cpp:84:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MutationEvent.h:34:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:49:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:50:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 7 in 81 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 297849 [details]
Patch

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

New failing tests:
fast/text/mark-matches-rendering.html
fast/text/mark-matches-broken-line-rendering.html
editing/text-iterator/count-mark-simple-lines.html
fast/text/mark-matches-overflow-clip.html
editing/text-iterator/count-mark-lineboxes.html

[Build Bot]

Created attachment 297850 [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 297849 [details]
Patch

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

New failing tests:
fast/text/mark-matches-rendering.html
fast/text/mark-matches-broken-line-rendering.html
editing/text-iterator/count-mark-simple-lines.html
fast/text/mark-matches-overflow-clip.html
editing/text-iterator/count-mark-lineboxes.html

[Build Bot]

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

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

[Build Bot]

Comment on attachment 297849 [details]
Patch

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

New failing tests:
editing/text-iterator/count-mark-simple-lines.html
fast/text/mark-matches-overflow-clip.html
editing/text-iterator/count-mark-lineboxes.html

[Build Bot]

Created attachment 297852 [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.11.6

[Build Bot]

Comment on attachment 297849 [details]
Patch

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

New failing tests:
editing/mac/spelling/click-autocorrected-word.html
editing/mac/spelling/delete-autocorrected-word-1.html
editing/mac/spelling/autocorrection-delete.html
imported/blink/editing/apply-inline-style-to-element-with-no-renderer-crash.html
fast/text/mark-matches-rendering.html
editing/text-iterator/count-mark-lineboxes.html
editing/mac/spelling/move-cursor-to-autocorrected-word.html
editing/mac/input/crash-for-empty-text-alternative.html
editing/mac/spelling/autocorrection-simple.html
editing/mac/spelling/delete-autocorrected-word-2.html
editing/mac/input/edit-dictated-text-with-alternative.html
editing/mac/spelling/autocorrection-with-multi-line-text.html
editing/mac/spelling/autocorrection-removing-underline.html
editing/mac/spelling/forward-delete-into-autocorrected-word.html
editing/mac/spelling/move-cursor-to-beginning-of-autocorrected-word.html
editing/mac/spelling/autocorrection-removing-underline-after-paste.html
editing/mac/spelling/autocorrection-blockquote-crash.html
fast/text/mark-matches-broken-line-rendering.html
editing/mac/spelling/removing-underline-after-accepting-autocorrection-using-punctuation.html
editing/text-iterator/count-mark-simple-lines.html
editing/mac/input/insert-dictated-text.html
fast/text/mark-matches-overflow-clip.html
editing/mac/spelling/delete-into-autocorrected-word.html
editing/mac/spelling/accept-candidate-allows-autocorrect-on-next-word.html
editing/mac/spelling/autocorrection-contraction.html

[Build Bot]

Created attachment 297853 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

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

[Build Bot]

Comment on attachment 297849 [details]
Patch

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

New failing tests:
editing/mac/spelling/click-autocorrected-word.html
editing/mac/spelling/delete-autocorrected-word-1.html
editing/mac/spelling/autocorrection-delete.html
imported/blink/editing/apply-inline-style-to-element-with-no-renderer-crash.html
fast/text/mark-matches-rendering.html
editing/text-iterator/count-mark-lineboxes.html
editing/mac/spelling/move-cursor-to-autocorrected-word.html
editing/mac/input/crash-for-empty-text-alternative.html
editing/mac/spelling/autocorrection-simple.html
editing/mac/spelling/delete-autocorrected-word-2.html
editing/mac/input/edit-dictated-text-with-alternative.html
editing/mac/spelling/autocorrection-with-multi-line-text.html
editing/mac/spelling/autocorrection-removing-underline.html
editing/mac/spelling/forward-delete-into-autocorrected-word.html
editing/mac/spelling/move-cursor-to-beginning-of-autocorrected-word.html
editing/mac/spelling/autocorrection-removing-underline-after-paste.html
editing/mac/spelling/autocorrection-blockquote-crash.html
fast/text/mark-matches-broken-line-rendering.html
editing/mac/spelling/removing-underline-after-accepting-autocorrection-using-punctuation.html
editing/text-iterator/count-mark-simple-lines.html
editing/mac/input/insert-dictated-text.html
fast/text/mark-matches-overflow-clip.html
editing/mac/spelling/delete-into-autocorrected-word.html
editing/mac/spelling/accept-candidate-allows-autocorrect-on-next-word.html
editing/mac/spelling/autocorrection-contraction.html

[Build Bot]

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

[Darin Adler]

Created attachment 297866 [details]
Patch

[WebKit Commit Bot]

Attachment 297866 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MouseEvent.cpp:84:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1034:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/dom/MutationEvent.h:34:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:35:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/dom/MutationEvent.h:36:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:49:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
ERROR: Source/WebCore/platform/text/StringWithDirection.h:50:  Should be indented on a separate line, with the colon or comma first on that line.  [whitespace/indent] [4]
Total errors found: 7 in 84 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 297866 [details]
Patch

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

> Source/WebCore/ChangeLog:458
> +        * rendering/HitTestResult.cpp:

This ChangeLog entry seems out of place.

> Source/WebCore/dom/DOMNamedFlowCollection.cpp:-67
> -    // FIXME: Should be implemented.

You implemented this.  Can it be tested?

> Source/WebCore/dom/DOMNamedFlowCollection.idl:39
> -    getter WebKitNamedFlow item(unsigned long index);
> -    getter WebKitNamedFlow namedItem(DOMString name);
> +    getter WebKitNamedFlow? item(unsigned long index);
> +    getter WebKitNamedFlow? namedItem(DOMString name);

Can this change be tested?

> Source/WebCore/dom/DocumentMarker.h:99
> +            : MarkerTypes(0

This 0 doesn't seem useful.

> Source/WebCore/editing/AlternativeTextController.cpp:731
>      Vector<RenderedDocumentMarker*> dictationAlternativesMarkers = markers.markersInRange(selection.get(), DocumentMarker::DictationAlternatives);

auto
Also, could we make markersInRange and markersFor return Vector<std::reference_wrapper<RenderedDocumentMarker>> to make it clear that they can't  be null?  If we only iterate them and count them, we might be able to get away with using clever iterator types and never allocating a Vector.

[Darin Adler]

Comment on attachment 297866 [details]
Patch

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

>> Source/WebCore/ChangeLog:458
>> +        * rendering/HitTestResult.cpp:
> 
> This ChangeLog entry seems out of place.

Oops! Will fix this.

>> Source/WebCore/dom/DOMNamedFlowCollection.cpp:-67
>> -    // FIXME: Should be implemented.
> 
> You implemented this.  Can it be tested?

Good point. I should definitely add a test even though I don’t already know how. I will figure it out.

>> Source/WebCore/dom/DOMNamedFlowCollection.idl:39
>> +    getter WebKitNamedFlow? namedItem(DOMString name);
> 
> Can this change be tested?

No, there is no change in behavior here. When we specify a return value that can’t be null in an IDL file, our generated code can handle null just fine, and vice versa, so we can get this wrong in IDL files with no repercussions.

>> Source/WebCore/dom/DocumentMarker.h:99
>> +            : MarkerTypes(0
> 
> This 0 doesn't seem useful.

It is here so that the subsequent lines can all have a "|" at the start of them, rather than having one special one without a "|".

>> Source/WebCore/editing/AlternativeTextController.cpp:731
>>      Vector<RenderedDocumentMarker*> dictationAlternativesMarkers = markers.markersInRange(selection.get(), DocumentMarker::DictationAlternatives);
> 
> auto
> Also, could we make markersInRange and markersFor return Vector<std::reference_wrapper<RenderedDocumentMarker>> to make it clear that they can't  be null?  If we only iterate them and count them, we might be able to get away with using clever iterator types and never allocating a Vector.

Sure, we could do those things. I prefer not to do them in this patch, though.

[Darin Adler]

Committed r210216: <http://trac.webkit.org/changeset/210216>