Fix layoutOverflowRect in case of explicit 'overflow: scroll', but no overflow content

Fix layoutOverflowRect in case of explicit 'overflow: scroll', but no overflow content.

BUG=371359
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174808

[pals, 2014-05-16 12:42:16]

The changes in RenderBox.cpp fixes the layoutOverflowRect initialization if
there is no overflow content. If there is no overflow content, the code return
from here 
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
so, so m_overflow was being wrongly initialized in here 
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit....

The changes in Source/core/rendering/RenderBlockLineLayout.cpp is to fix the
LayoutTests regressions.

[Julien - ping for review, 2014-05-19 14:36:18]

+Levi for the RenderBlockLineLayout which I think is fine but would need an
expert check.

https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
File LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html (right):

https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html:5: </div>
This test should use mock scrollbars to avoid being highly platform dependent. A
ref-test would be even better but I don't think is a good way to test this
change.

https://codereview.chromium.org/285293007/diff/10001/Source/core/rendering/Re...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/285293007/diff/10001/Source/core/rendering/Re...
Source/core/rendering/RenderBox.cpp:4273: m_overflow = adoptPtr(new
RenderOverflow(clientBoxRect(), borderBoxRect()));
This should be noOverflowRect() to match the rest of the code.

[pals, 2014-05-20 09:20:19]

Using mock scrollbars but still the pixel test becomes platform dependent. Am I
doing it correctly?

https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
File LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html (right):

https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html:5: </div>
On 2014/05/19 14:36:19, Julien Chaffraix - CET wrote:
> This test should use mock scrollbars to avoid being highly platform dependent.
A
> ref-test would be even better but I don't think is a good way to test this
> change.

Done.

https://codereview.chromium.org/285293007/diff/10001/Source/core/rendering/Re...
File Source/core/rendering/RenderBox.cpp (right):

https://codereview.chromium.org/285293007/diff/10001/Source/core/rendering/Re...
Source/core/rendering/RenderBox.cpp:4273: m_overflow = adoptPtr(new
RenderOverflow(clientBoxRect(), borderBoxRect()));
On 2014/05/19 14:36:19, Julien Chaffraix - CET wrote:
> This should be noOverflowRect() to match the rest of the code.

Done.

[Julien - ping for review, 2014-05-20 15:46:56]

lgtm

https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
File LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html (right):

https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html:2: <p>All the digits from 0
to 9 should be visible.</p>
The text is what makes the test platform-dependent. The WebKit wiki has a great
article on how to write platform-independent tests (it's down right now and we
should decide to write a similar page).

Basically to avoid that you should put:

html {
    font: 10px Ahem;
    -webkit-font-smoothing: none;
}

The output will be unreadable in DRT but that's expected and it avoids baseline
explosion.

[pals, 2014-05-21 06:52:09]

The CQ bit was checked by sanjoy.pal@samsung.com

[commit-bot: I haz the power, 2014-05-21 06:52:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sanjoy.pal@samsung.com/285293007/110001

[pals, 2014-05-21 13:57:37]

On 2014/05/20 15:46:56, Julien Chaffraix - CET wrote:
> lgtm
> 
>
https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
> File LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html (right):
> 
>
https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
> LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html:2: <p>All the digits from
0
> to 9 should be visible.</p>
> The text is what makes the test platform-dependent. The WebKit wiki has a
great
> article on how to write platform-independent tests (it's down right now and we
> should decide to write a similar page).
> 
> Basically to avoid that you should put:
> 
> html {
>     font: 10px Ahem;
>     -webkit-font-smoothing: none;
> }
> 
> The output will be unreadable in DRT but that's expected and it avoids
baseline
> explosion.

Still failing on MAC.

[Julien - ping for review, 2014-05-23 10:32:38]

On 2014/05/21 13:57:37, sanjoy.pal wrote:
> On 2014/05/20 15:46:56, Julien Chaffraix - CET wrote:
> > lgtm
> > 
> >
>
https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
> > File LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html (right):
> > 
> >
>
https://codereview.chromium.org/285293007/diff/10001/LayoutTests/scrollbars/r...
> > LayoutTests/scrollbars/rtl/overflow-scroll-rtl.html:2: <p>All the digits
from
> 0
> > to 9 should be visible.</p>
> > The text is what makes the test platform-dependent. The WebKit wiki has a
> great
> > article on how to write platform-independent tests (it's down right now and
we
> > should decide to write a similar page).
> > 
> > Basically to avoid that you should put:
> > 
> > html {
> >     font: 10px Ahem;
> >     -webkit-font-smoothing: none;
> > }
> > 
> > The output will be unreadable in DRT but that's expected and it avoids
> baseline
> > explosion.
> 
> Still failing on MAC.

Apparently the mock scrollbars are different on Mac
(https://storage.googleapis.com/chromium-layout-test-archives/mac_blink_rel/84...).
Not sure why that's the case.

+Some people who may know more about why mock scrollbars are different on Mac.

[Dirk Pranke, 2014-05-23 16:20:34]

The mac's mock scrollbars are rendered internally by webcore (they came from the
apple port originally). They are an entirely different implementation and
mechanism than the mock theme used by blink's win and linux ports.

Ideally we'd use the mock theme on all of our ports, but I've not yet found the
cycles to convert the mac over.

[pals, 2014-05-26 07:31:43]

The CQ bit was checked by sanjoy.pal@samsung.com

[commit-bot: I haz the power, 2014-05-26 07:31:56]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sanjoy.pal@samsung.com/285293007/170001

[commit-bot: I haz the power, 2014-05-26 07:32:07]

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

[commit-bot: I haz the power, 2014-05-26 07:32:07]

Failed to apply patch for
LayoutTests/platform/win/fast/block/float/026-expected.txt:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/platform/win/fast/block/float/026-expected.txt
  Hunk #1 FAILED at 38.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/platform/win/fast/block/float/026-expected.txt.rej

Patch:       LayoutTests/platform/win/fast/block/float/026-expected.txt
Index: LayoutTests/platform/win/fast/block/float/026-expected.txt
diff --git a/LayoutTests/platform/win/fast/block/float/026-expected.txt
b/LayoutTests/platform/win/fast/block/float/026-expected.txt
index
9e56c7226f74fbb14dc1945514faa518ef2f2b8a..8829abdf7ea23e02f07e3fa28c7d0fad4220165b
100644
--- a/LayoutTests/platform/win/fast/block/float/026-expected.txt
+++ b/LayoutTests/platform/win/fast/block/float/026-expected.txt
@@ -38,14 +38,14 @@ layer at (0,0) size 785x1432
         RenderImage {IMG} at (184.50,0) size 384.50x50 [bgcolor=#008000]
 layer at (8,208) size 569x59 clip at (25,210) size 550x40
   RenderBlock {DIV} at (0,0) size 569x59 [border: (2px solid #0000FF)]
-    RenderText {#text} at (39,2) size 513x39
-      text run at (39,2) width 513: "This is an overflow:scroll region. It
should sit on the same line as the float and wrap as"
-      text run at (290,22) width 4 RTL: "."
-      text run at (294,22) width 258: "needed to fit within the remaining line
width"
-layer at (189,816) size 388x79 clip at (206,818) size 369x60 scrollX 14
scrollWidth 383
+    RenderText {#text} at (54,2) size 513x39
+      text run at (54,2) width 513: "This is an overflow:scroll region. It
should sit on the same line as the float and wrap as"
+      text run at (305,22) width 4 RTL: "."
+      text run at (309,22) width 258: "needed to fit within the remaining line
width"
+layer at (189,816) size 388x79 clip at (206,818) size 369x60
   RenderBlock {DIV} at (180.50,0) size 388.50x79 [border: (2px solid #0000FF)]
-    RenderText {#text} at (5,2) size 368x59
-      text run at (5,2) width 366: "This is an overflow:scroll region. It
should sit on the same line"
-      text run at (3,22) width 368: "as the float and wrap as needed to fit
within the remaining line"
-      text run at (334,42) width 4 RTL: "."
-      text run at (338,42) width 33: "width"
+    RenderText {#text} at (20,2) size 368x59
+      text run at (20,2) width 366: "This is an overflow:scroll region. It
should sit on the same line"
+      text run at (18,22) width 368: "as the float and wrap as needed to fit
within the remaining line"
+      text run at (349,42) width 4 RTL: "."
+      text run at (353,42) width 33: "width"

[pals, 2014-05-26 12:01:02]

The CQ bit was checked by sanjoy.pal@samsung.com

[commit-bot: I haz the power, 2014-05-26 12:01:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sanjoy.pal@samsung.com/285293007/230001

[commit-bot: I haz the power, 2014-05-26 13:22:20]

Change committed as 174808