Bug 189035 - [LFC][Floating] Remove redundant FloatAvoider functions.
Summary: [LFC][Floating] Remove redundant FloatAvoider functions.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-27 16:39 PDT by zalan
Modified: 2018-08-28 08:03 PDT (History)
5 users (show)

See Also:

Attachments
Patch (13.56 KB, patch)
2018-08-27 16:46 PDT, zalan 		koivisto: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2018-08-27 16:39:53 PDT 
and move some code from FloatingContext to FloatAvoider
Comment 1 zalan 2018-08-27 16:46:02 PDT 
Created attachment 348239 [details]
Patch
Comment 2 Antti Koivisto 2018-08-28 07:26:28 PDT 
Comment on attachment 348239 [details]
Patch

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

> Source/WebCore/layout/floats/FloatAvoider.h:64
> +    void resetHorizontalConstraint();

Some of these are singulars like here "Constraint".

> Source/WebCore/layout/floats/FloatingContext.cpp:71
> +    FloatAvoider::HorizontalConstraints horizontalConstraints() const;

And some are plural. Is there some logic to this?
Comment 3 Antti Koivisto 2018-08-28 07:29:11 PDT 
Comment on attachment 348239 [details]
Patch

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

> Source/WebCore/layout/floats/FloatingContext.cpp:228
> +        floatAvoider.setHorizontalConstraints(floats.horizontalConstraints());
> +        floatAvoider.setVerticalConstraint(floats.verticalPosition());

It looks strange that the second one is reading floats.verticalPosition instead of floats.verticalConstraint. But maybe it makes sense?
Comment 4 zalan 2018-08-28 08:01:44 PDT 
Committed r235423: <https://trac.webkit.org/changeset/235423>
Comment 5 Radar WebKit Bug Importer 2018-08-28 08:03:10 PDT 
<rdar://problem/43797393>
Comment 6 zalan 2018-08-28 08:03:26 PDT 
(In reply to Antti Koivisto from comment #2)
> Comment on attachment 348239 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=348239&action=review
> 
> > Source/WebCore/layout/floats/FloatAvoider.h:64
> > +    void resetHorizontalConstraint();
> 
> Some of these are singulars like here "Constraint".
> 
> > Source/WebCore/layout/floats/FloatingContext.cpp:71
> > +    FloatAvoider::HorizontalConstraints horizontalConstraints() const;
> 
> And some are plural. Is there some logic to this?

Fixed. No logic, oversight.
Comment 7 Radar WebKit Bug Importer 2018-08-28 08:03:41 PDT 
<rdar://problem/43797406>
Comment 8 zalan 2018-08-28 08:03:45 PDT 
(In reply to Antti Koivisto from comment #3)
> Comment on attachment 348239 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=348239&action=review
> 
> > Source/WebCore/layout/floats/FloatingContext.cpp:228
> > +        floatAvoider.setHorizontalConstraints(floats.horizontalConstraints());
> > +        floatAvoider.setVerticalConstraint(floats.verticalPosition());
> 
> It looks strange that the second one is reading floats.verticalPosition
> instead of floats.verticalConstraint. But maybe it makes sense?
Fixed.
Comment 9 Radar WebKit Bug Importer 2018-08-28 08:03:57 PDT 
<rdar://problem/43797410>