Even more explicit LayoutUnit conversion

Even more explicit LayoutUnit conversion

Continued work toward making LayoutUnit's conversion constructors
explicit.

BUG=581254
TBR=senorblanco

Committed: https://crrev.com/e17f64a0e2379368cf9fd54109bbee246ca73b4f
Cr-Commit-Position: refs/heads/master@{#372561}

[leviw_travelin_and_unemployed, 2016-01-30 05:03:03]

leviw@chromium.org changed reviewers:
+ eae@chromium.org

[leviw_travelin_and_unemployed, 2016-01-30 05:03:03]

M0AR!!!!!!!

[eae, 2016-01-30 05:18:36]

LGTM with two suggestions

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:231: LayoutSize
minimumSize(LayoutUnit(size.width() > 0 ? 1 : 0), LayoutUnit(size.height() > 0 ?
1 : 0));
LayoutSize minimumSize(size.width() > LayoutUnit() ? LayoutUnit(1) : 0,
                       size.height() > 0 ? LayoutUnit(1) : LayoutUnit());

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (left):

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:1005: m_size =
LayoutSize(layoutSize().width(), layoutSize().height());
I thought we'd fixed all of these already!

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutMedia.cpp (right):

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutMedia.cpp:80:
layoutBox->setLocation(LayoutPoint(LayoutUnit(borderLeft()), borderTop()) +
LayoutSize(paddingLeft(), paddingTop()));
Isn't this exactly what contentBoxOffset() does (return border+padding)?
 
layoutBox->setLocation(contentBoxOffset());

[leviw_travelin_and_unemployed, 2016-01-30 05:36:07]

Thanks for the quick review!

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ImageResource.cpp (right):

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ImageResource.cpp:231: LayoutSize
minimumSize(LayoutUnit(size.width() > 0 ? 1 : 0), LayoutUnit(size.height() > 0 ?
1 : 0));
On 2016/01/30 at 05:18:36, eae wrote:
> LayoutSize minimumSize(size.width() > LayoutUnit() ? LayoutUnit(1) : 0,
>                        size.height() > 0 ? LayoutUnit(1) : LayoutUnit());

Works for me :)

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (left):

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:1005: m_size =
LayoutSize(layoutSize().width(), layoutSize().height());
On 2016/01/30 at 05:18:36, eae wrote:
> I thought we'd fixed all of these already!

Crazy, right?! People still write this crap ;)

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutMedia.cpp (right):

https://codereview.chromium.org/1653673002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutMedia.cpp:80:
layoutBox->setLocation(LayoutPoint(LayoutUnit(borderLeft()), borderTop()) +
LayoutSize(paddingLeft(), paddingTop()));
On 2016/01/30 at 05:18:36, eae wrote:
> Isn't this exactly what contentBoxOffset() does (return border+padding)?
>  
> layoutBox->setLocation(contentBoxOffset());

Awesome point :D

[eae, 2016-01-30 05:40:22]

\o/
 |
/\

[leviw_travelin_and_unemployed, 2016-01-30 06:13:07]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2016-01-30 06:13:07]

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

[commit-bot: I haz the power, 2016-01-30 06:13:41]

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

[commit-bot: I haz the power, 2016-01-30 06:13:42]

This CL has an open dependency (Issue 1647313003 Patch 60001). Please resolve
the dependency and try again.

[leviw_travelin_and_unemployed, 2016-01-30 06:17:32]

The CQ bit was checked by leviw@chromium.org

[commit-bot: I haz the power, 2016-01-30 06:17:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1653673002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1653673002/40001

[commit-bot: I haz the power, 2016-01-30 06:26:17]

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

[commit-bot: I haz the power, 2016-01-30 06:26:18]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[leviw_travelin_and_unemployed, 2016-01-30 06:28:16]

leviw@chromium.org changed reviewers:
+ senorblanco@chromium.org

[leviw_travelin_and_unemployed, 2016-01-30 22:10:47]

Description was changed from

==========
Even more explicit LayoutUnit conversion

Continued work toward making LayoutUnit's conversion constructors
explicit.

BUG=581254
==========

to

==========
Even more explicit LayoutUnit conversion

Continued work toward making LayoutUnit's conversion constructors
explicit.

BUG=581254
TBR=senorblanco
==========

[leviw_travelin_and_unemployed, 2016-01-30 22:11:06]

The CQ bit was checked by leviw@chromium.org

[leviw_travelin_and_unemployed, 2016-01-30 22:11:06]

TBR-ing the tiny canvas change.

[commit-bot: I haz the power, 2016-01-30 22:11:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1653673002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1653673002/40001

[commit-bot: I haz the power, 2016-01-30 22:16:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-30 22:16:56]

Description was changed from

==========
Even more explicit LayoutUnit conversion

Continued work toward making LayoutUnit's conversion constructors
explicit.

BUG=581254
TBR=senorblanco
==========

to

==========
Even more explicit LayoutUnit conversion

Continued work toward making LayoutUnit's conversion constructors
explicit.

BUG=581254
TBR=senorblanco

Committed: https://crrev.com/e17f64a0e2379368cf9fd54109bbee246ca73b4f
Cr-Commit-Position: refs/heads/master@{#372561}
==========

[commit-bot: I haz the power, 2016-01-30 22:16:58]

Patchset 3 (id:??) landed as
https://crrev.com/e17f64a0e2379368cf9fd54109bbee246ca73b4f
Cr-Commit-Position: refs/heads/master@{#372561}