[webkit-reviews] review granted: [Bug 25969] REGRESSION (r42334): Unnecessary scroll bars remain after the document shrinks : [Attachment 30670] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dave Hyatt <hyatt at apple.com>'s
request for review:
Bug 25969: REGRESSION (r42334): Unnecessary scroll bars remain after the
document shrinks
https://bugs.webkit.org/show_bug.cgi?id=25969

Attachment 30670: Patch
https://bugs.webkit.org/attachment.cgi?id=30670&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   Added two tests in fast/dynamic.

It's better to give the names of the tests.

> -- (void)_boundsChanged
> +- (void)_boundsChangedToNewSize:(NSSize)size

I think _boundsChangedTo: would be fine here rather than
_boundsChangedToNewSize:.

> -    if (!NSEqualSizes(_private->lastLayoutSize, [self bounds].size)) {
> +	if (!NSEqualSizes(_private->lastLayoutSize, size)) {

Indented one extra space here.

> -	       frame->view()->resize([self bounds].size.width, [self
bounds].size.height);
> +	       frame->view()->resize(size.width, size.height);

How about just resize(IntSize(size)) here instead? Has to be an explicit
conversion because it's a conversion from floating point to integer, but nice
to leave it as an object instead of breaking up into width/height.

> +- (void)setFrameSize:(NSSize)size
> +{
> +    [self _boundsChangedToNewSize:size];
> +    [super setFrameSize:size];
> +}

I'm not sure we need a separate _boundsChangedToNewSize: method. I'd just put
that code into the setFrameSize: override and not have a separate method.

r=me