[webkit-reviews] review granted: [Bug 225577] Remove uses of the String::toInt family of functions from WebCore/html and similar directories : [Attachment 428115] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 225577: Remove uses of the String::toInt family of functions from
WebCore/html and similar directories
https://bugs.webkit.org/show_bug.cgi?id=225577

Attachment 428115: Patch

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




--- Comment #2 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 428115
  --> https://bugs.webkit.org/attachment.cgi?id=428115
Patch

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

> Source/WebCore/html/FTPDirectoryDocument.cpp:169
> +	   return makeString(FormattedNumber::fixedWidth(*bytes / 1000., 2), "
KB");

I am not sure if our style guide has anything to say about it, but I am not a
fan of the lack of a trailing 0 here. I much prefer "1000.0" to "1000.". Same
below.

> Source/WebCore/html/FormController.cpp:69
> -    size_t size = stateVector[index++].toUInt();
> +    auto size =
parseIntegerAllowingTrailingJunk<size_t>(stateVector[index++]).valueOr(0);

I realize this is keeping existing behavior, but it seems like returning
WTF::nullopt on failure (and perhaps not allowing trailing junk as well) would
be better here since I believe this is all state that we are serializing and
expect to be consistent (though I will admit to not being completely familiar
with this. 

That said, this could (and probably should) be done separately.

> Source/WebCore/html/HTMLFrameSetElement.cpp:135
> -	       m_border = value.toInt();
> +	       m_border =
parseIntegerAllowingTrailingJunk<int>(value).valueOr(0);

Again, not to be changed here, and what this patch is doing that is so great is
really showing all the places where we should re-evaluate things, but it seems
suspect that setting <frameset border="apple"> behaves potentially differently
than <frameset> due to	m_borderSet being set to true.