Pass the correct BorderWidth value when creating BorderValues

Pass the correct BorderWidth value when creating BorderValues

This patch fixes two bugs introduced by me when splitting up
border-*-{width,style} out of BorderValue.

1. When creating a BorderValue to pass to layout code in ComputedStyle
via Border{Left,Right,Bottom,Top,Start,End}() we should be creating
these using Border*WidthInternal getters and not the public Border*Width
getters as that is what the code did before and the public getters
return a 0 width in case the Border*Style is none or hidden. This issue
wasn't discovered due to the fact that there were no unit/layout tests.
This patch adds these tests.

Future work will be to wrap all the float <-> unsigned conversions in a
class of its own.

https: //codereview.chromium.org/2890733002. I mistakenly swapped a
Review-Url: https://codereview.chromium.org/2895933004
Cr-Commit-Position: refs/heads/master@{#473836}
Committed: https://chromium.googlesource.com/chromium/src/+/3363d5074a1b4579a8ccc9ddd1e52f99489719fe

[nainar, 2017-05-23 04:55:06]

The CQ bit was checked by nainar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 04:55:21]

Dry run: CQ is trying da patch.

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

[nainar, 2017-05-23 05:05:58]

The CQ bit was checked by nainar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 05:06:11]

Dry run: CQ is trying da patch.

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

[nainar, 2017-05-23 05:06:49]

nainar@chromium.org changed reviewers:
+ shend@chromium.org

[nainar, 2017-05-23 05:06:50]

https://codereview.chromium.org/2895933004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/style/ComputedStyleTest.cpp (right):

https://codereview.chromium.org/2895933004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleTest.cpp:105:
TEST(ComputedStyleTest, HasOutlineWithCurrentColor) {
These were spelt incorrectly and never ran before.

https://codereview.chromium.org/2895933004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/style/ComputedStyleTest.cpp:129:
RefPtr<ComputedStyle> style = ComputedStyle::Create();
Happy to copy this test for all Border*Width.

[shend, 2017-05-23 06:12:06]

On 2017/05/23 at 05:06:50, nainar wrote:
>
https://codereview.chromium.org/2895933004/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/style/ComputedStyleTest.cpp (right):
> 
>
https://codereview.chromium.org/2895933004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/style/ComputedStyleTest.cpp:105:
TEST(ComputedStyleTest, HasOutlineWithCurrentColor) {
> These were spelt incorrectly and never ran before.
> 
>
https://codereview.chromium.org/2895933004/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/style/ComputedStyleTest.cpp:129:
RefPtr<ComputedStyle> style = ComputedStyle::Create();
> Happy to copy this test for all Border*Width.

lgtm. Not sure about how exhaustive unit tests should be. Ideally we'd be
generating tests for getters and setters to cover all of them.

[nainar, 2017-05-23 06:14:03]

nainar@chromium.org changed reviewers:
+ alancutter@chromium.org

[nainar, 2017-05-23 06:14:04]

alancutter@, 

PTAL? Thanks! Also let me know if the unit test isnt exahuastive enough

[commit-bot: I haz the power, 2017-05-23 06:41:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-23 06:41:50]

Dry run: This issue passed the CQ dry run.

[alancutter (OOO until 2018), 2017-05-23 08:12:23]

lgtm

[nainar, 2017-05-23 08:13:18]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2017-05-23 08:13:37]

CQ is trying da patch.

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

[commit-bot: I haz the power, 2017-05-23 08:19:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495527198401540,
"parent_rev": "62eb5de09e830c95b1d27688d2bac91a6390f0e7", "commit_rev":
"3363d5074a1b4579a8ccc9ddd1e52f99489719fe"}

[commit-bot: I haz the power, 2017-05-23 08:19:48]

Description was changed from

==========
Pass the correct BorderWidth value when creating BorderValues

This patch fixes two bugs introduced by me when splitting up
border-*-{width,style} out of BorderValue.

1. When creating a BorderValue to pass to layout code in ComputedStyle
via Border{Left,Right,Bottom,Top,Start,End}() we should be creating
these using Border*WidthInternal getters and not the public Border*Width
getters as that is what the code did before and the public getters
return a 0 width in case the Border*Style is none or hidden. This issue
wasn't discovered due to the fact that there were no unit/layout tests.
This patch adds these tests.

Future work will be to wrap all the float <-> unsigned conversions in a
class of its own.

2. When making EBorderStyle an enum class in
https://codereview.chromium.org/2890733002. I mistakenly swapped a
line incorrectly here:
https://codereview.chromium.org/2890733002/diff/20001/third_party/WebKit/Sour....
This issue meant that we were storing a width even if the BorderStyle
was hidden. This issue should have been caught by the
CollapsedBorderValueTest.Hidden but wasn't due to the previous bug.
No tests have been added as there is currenly sufficient test coverage.
==========

to

==========
Pass the correct BorderWidth value when creating BorderValues

This patch fixes two bugs introduced by me when splitting up
border-*-{width,style} out of BorderValue.

1. When creating a BorderValue to pass to layout code in ComputedStyle
via Border{Left,Right,Bottom,Top,Start,End}() we should be creating
these using Border*WidthInternal getters and not the public Border*Width
getters as that is what the code did before and the public getters
return a 0 width in case the Border*Style is none or hidden. This issue
wasn't discovered due to the fact that there were no unit/layout tests.
This patch adds these tests.

Future work will be to wrap all the float <-> unsigned conversions in a
class of its own.

https: //codereview.chromium.org/2890733002. I mistakenly swapped a
Review-Url: https://codereview.chromium.org/2895933004
Cr-Commit-Position: refs/heads/master@{#473836}
Committed:
https://chromium.googlesource.com/chromium/src/+/3363d5074a1b4579a8ccc9ddd1e5...
==========

[commit-bot: I haz the power, 2017-05-23 08:19:49]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/3363d5074a1b4579a8ccc9ddd1e5...