[webkit-reviews] review granted: [Bug 175028] Use references rather than pointers for register/unregister functions, and more : [Attachment 359623] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 175028: Use references rather than pointers for register/unregister
functions, and more
https://bugs.webkit.org/show_bug.cgi?id=175028

Attachment 359623: Patch

https://bugs.webkit.org/attachment.cgi?id=359623&action=review




--- Comment #13 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 359623
  --> https://bugs.webkit.org/attachment.cgi?id=359623
Patch

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

I get the sense you were holding back from making more changes to keep this
patch focused on the task at hand. I know I had difficulty keeping myself from
commenting on other nits. I couldn’t help myself in the few comments I did
make. Oops! Feel free to disregard.

> Source/WebCore/dom/Document.h:1600
> +    void buildAccessKeyMap(TreeScope& root);

Parameter name doesn’t add much value to me :/

> Source/WebCore/dom/DocumentMarkerController.cpp:65
> +    for (TextIterator markedText(&range); !markedText.atEnd();
markedText.advance()) {

Ok as-is. Be a bit more idiomatic looking if we use assignment notation to
invoke the constructor when initializing markedText.

> Source/WebCore/dom/DocumentMarkerController.cpp:104
> +    for (TextIterator markedText(&range); !markedText.atEnd();
markedText.advance()) {

Ditto.

> Source/WebCore/dom/DocumentMarkerController.cpp:117
> +    for (TextIterator markedText(&range); !markedText.atEnd();
markedText.advance()) {

Ditto

> Source/WebCore/dom/DocumentMarkerController.cpp:128
> +    for (TextIterator markedText(&range); !markedText.atEnd();
markedText.advance()) {

Ditto

> Source/WebCore/dom/DocumentMarkerController.cpp:138
> +    for (TextIterator markedText(&range); !markedText.atEnd();
markedText.advance()) {

Ditto