Use LayoutBox::frameRect() as the viewport of LayoutSVGForeignObject

Use LayoutBox::frameRect() as the viewport of LayoutSVGForeignObject

LayoutBox::frameRect() and LayoutSVGForeignObject::viewportRect()
have the same value, so we can just use the former.

Also fixed vertical writing mode bug.

This make it easier to let SVGText behave in a similar way.

BUG=666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/82ac602161c3c94c1c7dc979339a2908f71e71ca
Cr-Commit-Position: refs/heads/master@{#433431}

[Xianzhu, 2016-11-18 22:34:18]

Description was changed from

==========
Use LayoutBox::frameRect() as the viewport of LayoutSVGForeignObject

LayoutBox::frameRect() and LayoutSVGForeignObject::viewportRect()
have the same value, so we can just use the former.

Also fixed vertical writing mode bug.

This make it easier to let SVGText behave in a similar way.

BUG=666416
==========

to

==========
Use LayoutBox::frameRect() as the viewport of LayoutSVGForeignObject

LayoutBox::frameRect() and LayoutSVGForeignObject::viewportRect()
have the same value, so we can just use the former.

Also fixed vertical writing mode bug.

This make it easier to let SVGText behave in a similar way.

BUG=666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[Xianzhu, 2016-11-18 22:38:06]

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

[Xianzhu, 2016-11-18 22:38:15]

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

[commit-bot: I haz the power, 2016-11-18 22:39:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xianzhu, 2016-11-18 22:47:28]

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

[commit-bot: I haz the power, 2016-11-18 22:48:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[pdr., 2016-11-18 23:23:10]

https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp
(right):

https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp:115:
setX(elementX());
Does this also need to include writing mode?

Something like setLogicalLeft(isHoriz ? elementX : elementY);

[Xianzhu, 2016-11-18 23:54:00]

https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp
(right):

https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp:115:
setX(elementX());
On 2016/11/18 23:23:10, pdr. wrote:
> Does this also need to include writing mode?
> 
> Something like setLogicalLeft(isHoriz ? elementX : elementY);

No. We would need that if we called setLogicalX() and setLogicalY().

[Xianzhu, 2016-11-18 23:55:53]

On 2016/11/18 23:54:00, Xianzhu wrote:
>
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp
> (right):
> 
>
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp:115:
> setX(elementX());
> On 2016/11/18 23:23:10, pdr. wrote:
> > Does this also need to include writing mode?
> > 
> > Something like setLogicalLeft(isHoriz ? elementX : elementY);
> 
> No. We would need that if we called setLogicalX() and setLogicalY().

I mean
  setLogicalLeft(isHoriz ? elementX : elementY)
and
  setLogicalTop(isHoriz ? elementY : elementX)
are actually equivalent to
  setX(elementX)
and
  setY(elementY).

[pdr., 2016-11-19 00:01:54]

On 2016/11/18 at 23:55:53, wangxianzhu wrote:
> On 2016/11/18 23:54:00, Xianzhu wrote:
> >
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp
> > (right):
> > 
> >
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp:115:
> > setX(elementX());
> > On 2016/11/18 23:23:10, pdr. wrote:
> > > Does this also need to include writing mode?
> > > 
> > > Something like setLogicalLeft(isHoriz ? elementX : elementY);
> > 
> > No. We would need that if we called setLogicalX() and setLogicalY().
> 
> I mean
>   setLogicalLeft(isHoriz ? elementX : elementY)
> and
>   setLogicalTop(isHoriz ? elementY : elementX)
> are actually equivalent to
>   setX(elementX)
> and
>   setY(elementY).

I see. Is same is true for setLogicalWidth, but because it's called in
updateLogicalWidth and computeLogicalHeight, we can't call setWidth/setHeight
directly?

[Xianzhu, 2016-11-19 00:05:21]

On 2016/11/19 00:01:54, pdr. wrote:
> On 2016/11/18 at 23:55:53, wangxianzhu wrote:
> > On 2016/11/18 23:54:00, Xianzhu wrote:
> > >
>
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp
> > > (right):
> > > 
> > >
>
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp:115:
> > > setX(elementX());
> > > On 2016/11/18 23:23:10, pdr. wrote:
> > > > Does this also need to include writing mode?
> > > > 
> > > > Something like setLogicalLeft(isHoriz ? elementX : elementY);
> > > 
> > > No. We would need that if we called setLogicalX() and setLogicalY().
> > 
> > I mean
> >   setLogicalLeft(isHoriz ? elementX : elementY)
> > and
> >   setLogicalTop(isHoriz ? elementY : elementX)
> > are actually equivalent to
> >   setX(elementX)
> > and
> >   setY(elementY).
> 
> I see. Is same is true for setLogicalWidth, but because it's called in
> updateLogicalWidth and computeLogicalHeight, we can't call setWidth/setHeight
> directly?

Right. In vertical writing mode, the previous code called setWidth() in
updateLogicalWidth(), and the value calculated in computeLogicalHeight() is also
passed to setWidth() by the caller, resulting wrong width and zero height.

[pdr., 2016-11-19 00:07:06]

On 2016/11/19 at 00:05:21, wangxianzhu wrote:
> On 2016/11/19 00:01:54, pdr. wrote:
> > On 2016/11/18 at 23:55:53, wangxianzhu wrote:
> > > On 2016/11/18 23:54:00, Xianzhu wrote:
> > > >
> >
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> > > > File
third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp
> > > > (right):
> > > > 
> > > >
> >
https://codereview.chromium.org/2510313004/diff/40001/third_party/WebKit/Sour...
> > > >
third_party/WebKit/Source/core/layout/svg/LayoutSVGForeignObject.cpp:115:
> > > > setX(elementX());
> > > > On 2016/11/18 23:23:10, pdr. wrote:
> > > > > Does this also need to include writing mode?
> > > > > 
> > > > > Something like setLogicalLeft(isHoriz ? elementX : elementY);
> > > > 
> > > > No. We would need that if we called setLogicalX() and setLogicalY().
> > > 
> > > I mean
> > >   setLogicalLeft(isHoriz ? elementX : elementY)
> > > and
> > >   setLogicalTop(isHoriz ? elementY : elementX)
> > > are actually equivalent to
> > >   setX(elementX)
> > > and
> > >   setY(elementY).
> > 
> > I see. Is same is true for setLogicalWidth, but because it's called in
> > updateLogicalWidth and computeLogicalHeight, we can't call
setWidth/setHeight
> > directly?
> 
> Right. In vertical writing mode, the previous code called setWidth() in
updateLogicalWidth(), and the value calculated in computeLogicalHeight() is also
passed to setWidth() by the caller, resulting wrong width and zero height.

LGTM

[Xianzhu, 2016-11-19 00:46:14]

The CQ bit was unchecked by wangxianzhu@chromium.org

[Xianzhu, 2016-11-19 00:46:18]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-11-19 00:47:03]

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, 2016-11-19 00:49:29]

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

[commit-bot: I haz the power, 2016-11-19 00:49:30]

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Xianzhu, 2016-11-19 00:50:13]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-11-19 00:51:24]

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, 2016-11-19 02:14:30]

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

[commit-bot: I haz the power, 2016-11-19 02:14:31]

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

[Xianzhu, 2016-11-19 20:17:58]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-11-19 20:17:58]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2510313004/#ps60001
(title: "crbug.com/667071")

[commit-bot: I haz the power, 2016-11-19 20:18:10]

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, 2016-11-19 23:58:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1479586678102430,
"parent_rev": ["fe97853c0d16b21c9e9864401c19f50058cc8f81", null], "commit_rev":
["df276c6c2e3302a14d9169a7327958e0fe5d8418", null]}

[commit-bot: I haz the power, 2016-11-19 23:58:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-11-20 00:01:10]

Description was changed from

==========
Use LayoutBox::frameRect() as the viewport of LayoutSVGForeignObject

LayoutBox::frameRect() and LayoutSVGForeignObject::viewportRect()
have the same value, so we can just use the former.

Also fixed vertical writing mode bug.

This make it easier to let SVGText behave in a similar way.

BUG=666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Use LayoutBox::frameRect() as the viewport of LayoutSVGForeignObject

LayoutBox::frameRect() and LayoutSVGForeignObject::viewportRect()
have the same value, so we can just use the former.

Also fixed vertical writing mode bug.

This make it easier to let SVGText behave in a similar way.

BUG=666416
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/82ac602161c3c94c1c7dc979339a2908f71e71ca
Cr-Commit-Position: refs/heads/master@{#433431}
==========

[commit-bot: I haz the power, 2016-11-20 00:01:12]

Patchset 4 (id:??) landed as
https://crrev.com/82ac602161c3c94c1c7dc979339a2908f71e71ca
Cr-Commit-Position: refs/heads/master@{#433431}