Move condition in CollapsedBorderValue::Width() into constructor

Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from being repeatedly checked.

Also tweek other methods to avoid unnecessary checks. Add test.

BUG=717128
Review-Url: https://codereview.chromium.org/2858143003
Cr-Commit-Position: refs/heads/master@{#469562}
Committed: https://chromium.googlesource.com/chromium/src/+/bfbe9c7802a991ba8aac2664970947fd18e4748c

[Xianzhu, 2017-05-04 17:14:19]

Description was changed from

==========
Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from repeatedly checked.

BUG=717128
==========

to

==========
Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from being repeatedly checked.

BUG=717128
==========

[Xianzhu, 2017-05-04 20:45:25]

wangxianzhu@chromium.org changed reviewers:
+ wkorman@chromium.org

[Xianzhu, 2017-05-04 20:45:26]

https://codereview.chromium.org/2858143003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/CollapsedBorderValue.h (right):

https://codereview.chromium.org/2858143003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/CollapsedBorderValue.h:59:
width_(border.Style() > kBorderStyleHidden ? border.Width() : 0),
BorderValue::NonZero() is:
 { return Width() && (style_ != kBorderStyleNone); }

width_ was and will be set to:
                               old code         new code
border.Width()==0                 0                0
Style()==kBorderStyleNone         0                0
Style()==kBorderStyleHidden    border.Width()      0

For both old code and new code, CollapsedBorderValue::Width() returns 0 in the
above cases, or otherwise the original border width.

[wkorman, 2017-05-04 21:13:04]

lgtm

https://codereview.chromium.org/2858143003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/CollapsedBorderValue.h (right):

https://codereview.chromium.org/2858143003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/CollapsedBorderValue.h:59:
width_(border.Style() > kBorderStyleHidden ? border.Width() : 0),
On 2017/05/04 20:45:26, Xianzhu wrote:
> BorderValue::NonZero() is:
>  { return Width() && (style_ != kBorderStyleNone); }
> 
> width_ was and will be set to:
>                                old code         new code
> border.Width()==0                 0                0
> Style()==kBorderStyleNone         0                0
> Style()==kBorderStyleHidden    border.Width()      0
> 
> For both old code and new code, CollapsedBorderValue::Width() returns 0 in the
> above cases, or otherwise the original border width.

Seems like unit tests for these permutations could be helpful for future. It is
mainly a simple container class but there is some logic here and the class is
referenced by both LayoutTableCell and TableSectionPainter.

[Xianzhu, 2017-05-04 22:23:01]

Description was changed from

==========
Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from being repeatedly checked.

BUG=717128
==========

to

==========
Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from being repeatedly checked.

Also tweek other methods to avoid unnecessary checks. Add test.

BUG=717128
==========

[Xianzhu, 2017-05-04 22:23:06]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2017-05-04 22:23:07]

The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org
Link to the patchset: https://codereview.chromium.org/2858143003/#ps40001
(title: "-")

[commit-bot: I haz the power, 2017-05-04 22:23:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xianzhu, 2017-05-04 22:23:35]

https://codereview.chromium.org/2858143003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/CollapsedBorderValue.h (right):

https://codereview.chromium.org/2858143003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/CollapsedBorderValue.h:59:
width_(border.Style() > kBorderStyleHidden ? border.Width() : 0),
On 2017/05/04 21:13:04, wkorman wrote:
> On 2017/05/04 20:45:26, Xianzhu wrote:
> > BorderValue::NonZero() is:
> >  { return Width() && (style_ != kBorderStyleNone); }
> > 
> > width_ was and will be set to:
> >                                old code         new code
> > border.Width()==0                 0                0
> > Style()==kBorderStyleNone         0                0
> > Style()==kBorderStyleHidden    border.Width()      0
> > 
> > For both old code and new code, CollapsedBorderValue::Width() returns 0 in
the
> > above cases, or otherwise the original border width.
> 
> Seems like unit tests for these permutations could be helpful for future. It
is
> mainly a simple container class but there is some logic here and the class is
> referenced by both LayoutTableCell and TableSectionPainter.

Added tests.

[commit-bot: I haz the power, 2017-05-05 01:16:42]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493936586810250,
"parent_rev": "b1c163ea6bf56b21576bed89fcf4162eebb2ab91", "commit_rev":
"bfbe9c7802a991ba8aac2664970947fd18e4748c"}

[commit-bot: I haz the power, 2017-05-05 01:16:57]

Description was changed from

==========
Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from being repeatedly checked.

Also tweek other methods to avoid unnecessary checks. Add test.

BUG=717128
==========

to

==========
Move condition in CollapsedBorderValue::Width() into constructor

This avoids the condition from being repeatedly checked.

Also tweek other methods to avoid unnecessary checks. Add test.

BUG=717128

Review-Url: https://codereview.chromium.org/2858143003
Cr-Commit-Position: refs/heads/master@{#469562}
Committed:
https://chromium.googlesource.com/chromium/src/+/bfbe9c7802a991ba8aac26649709...
==========

[commit-bot: I haz the power, 2017-05-05 01:16:58]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/bfbe9c7802a991ba8aac26649709...