Bug 85325 – Remove uneeded min/max pref width assignment from RenderView

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Remove uneeded min/max pref width assignment from RenderView

[Eric Seidel (no email)]

Created attachment 139712 [details]
Patch

[Eric Seidel (no email)]

With cr-linux (thus the layout tests) passing, this should just be a rubber-stamp for anyone willing.

[Emil A Eklund]

Comment on attachment 139712 [details]
Patch

Seems reasonable, would be good to eventually get rid of it though!

[Levi Weintraub]

Comment on attachment 139712 [details]
Patch

Great description and I agree with your conclusion. LGTM.

[Eric Seidel (no email)]

Of course we need an actual reviewer to pull the trigger...

[Julien Chaffraix]

Comment on attachment 139712 [details]
Patch

It sounds like we should annotate the function to say that we should remove it and have people not relying on it.

[Eric Seidel (no email)]

Created attachment 139717 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 139717 [details]
Patch for landing

Clearing flags on attachment: 139717

Committed r115779: <http://trac.webkit.org/changeset/115779>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 139717 [details]
Patch for landing

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

> Source/WebCore/rendering/RenderView.h:60
> +    // FIXME: This override is not needed and should be removed
> +    // it only exists to make computePreferredLogicalWidths public.
> +    virtual void computePreferredLogicalWidths() OVERRIDE;

The way to do that is:

    using RenderBlock::computePreferredLogicalWidths;

That makes computePreferredLogicalWidths public in RenderView without adding an override.

[Eric Seidel (no email)]

(In reply to comment #10)
> (From update of attachment 139717 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=139717&action=review
> 
> > Source/WebCore/rendering/RenderView.h:60
> > +    // FIXME: This override is not needed and should be removed
> > +    // it only exists to make computePreferredLogicalWidths public.
> > +    virtual void computePreferredLogicalWidths() OVERRIDE;
> 
> The way to do that is:
> 
>     using RenderBlock::computePreferredLogicalWidths;
> 
> That makes computePreferredLogicalWidths public in RenderView without adding an override.

Cute.  I wasn't aware of that aspect of c++.

It's unclear that this function even needs to exist.  I believe removing it has a small chance of breaking some (possibly untested) frame flattening code paths, so I've left it for now.  I would like to go back and see about removing this function entirely (as I had on my branch), once I'm done landing seamless.