[New Multicolumn] Support for paged overflow. (By mstensho).

[New Multicolumn] Support for paged overflow. (By mstensho).

This brings support for overflow-y:-webkit-paged-[xy] to the new
multicol implementation.

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

[rune, 2014-07-03 22:22:17]

This is https://codereview.chromium.org/295373006/ now owned by andersr. I
review the current code there. Issues/comments there still apply.

[rune, 2014-07-04 09:44:26]

https://codereview.chromium.org/364233005/diff/30090/Source/core/rendering/Re...
File Source/core/rendering/RenderBlockFlow.cpp (right):

https://codereview.chromium.org/364233005/diff/30090/Source/core/rendering/Re...
Source/core/rendering/RenderBlockFlow.cpp:2773: if ((type != NoFlowThread) &&
!multiColumnFlowThread()) {
The blink code often use early returns. How about:

if (type == NoFlowThread || multiColumnFlowThread())
    return;

Gets rid of the double negation, less indentation, and more readable perhaps.

[andersr, 2014-07-04 09:49:57]

The CQ bit was checked by andersr@opera.com

[commit-bot: I haz the power, 2014-07-04 09:50:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/andersr@opera.com/364233005/30090

[commit-bot: I haz the power, 2014-07-04 09:50:35]

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

[commit-bot: I haz the power, 2014-07-04 09:50:36]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[andersr, 2014-07-04 09:55:28]

On 2014/07/04 09:50:36, I haz the power (commit-bot) wrote:
> No LGTM from a valid reviewer yet.

Yes, commit-bot, I know.

I ticked the box to run tests on "other" platforms (I have a specific reason to
believe some might fail), but I guess that's not possible anymore.

[rune, 2014-07-04 12:30:53]

lgtm

https://codereview.chromium.org/364233005/diff/90001/Source/core/rendering/Re...
File Source/core/rendering/RenderBlockFlow.cpp (right):

https://codereview.chromium.org/364233005/diff/90001/Source/core/rendering/Re...
Source/core/rendering/RenderBlockFlow.cpp:2791: RenderMultiColumnFlowThread*
flowThread = createMultiColumnFlowThread(type);
Sweet.

[andersr, 2014-07-04 12:35:00]

The CQ bit was checked by andersr@opera.com

[commit-bot: I haz the power, 2014-07-04 12:35:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/andersr@opera.com/364233005/90001

[andersr, 2014-07-04 13:21:10]

The CQ bit was unchecked by andersr@opera.com

[andersr, 2014-07-04 14:30:38]

+dpranke, could you please review the changes to the virtual test suite?

We need to run fast/pagination in the new multicol implementation as well, now
that we have two tests in there that require it.

rune: All the tests in fast/pagination passed in the new implementation, so I'm
only skipping the tests added in this patch during the regular execution of
fast/multicol and fast/pagination.

[rune, 2014-07-04 15:15:21]

On 2014/07/04 14:30:38, andersr wrote:

> rune: All the tests in fast/pagination passed in the new implementation, so
I'm
> only skipping the tests added in this patch during the regular execution of
> fast/multicol and fast/pagination.

And you did that because the added tests only pass in the new implementation
because the new implementation fixes a bug?

[andersr, 2014-07-04 15:40:05]

On 2014/07/04 15:15:21, rune (futhark.irc - CET) wrote:
> On 2014/07/04 14:30:38, andersr wrote:
> 
> > rune: All the tests in fast/pagination passed in the new implementation, so
> I'm
> > only skipping the tests added in this patch during the regular execution of
> > fast/multicol and fast/pagination.
> 
> And you did that because the added tests only pass in the new implementation
> because the new implementation fixes a bug?

Just one (out of four) of the new tests failed in the old implementation.

I skipped all of them because that's what mstensho's original patch in effect
did. I don't know if the new tests are valid in the old implementation, or if
they just pass by coincidence.

If you prefer to enable all "new" (currently skipped) tests that happen to pass
in fast/[multicol, pagination], then it might as well be in a follow-up commit,
because there's a whole bunch of them in TestExpectations already. Personally,
though, I think we should spend as little time as possible on old multicol.

[jochen (gone - plz use gerrit), 2014-07-08 08:47:56]

base.py lgtm

[andersr, 2014-07-08 08:50:04]

The CQ bit was checked by andersr@opera.com

[commit-bot: I haz the power, 2014-07-08 08:50:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/andersr@opera.com/364233005/110001

[commit-bot: I haz the power, 2014-07-08 09:53:43]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_blink_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_re...)

[commit-bot: I haz the power, 2014-07-08 10:03:37]

Change committed as 177654

[Dirk Pranke, 2014-07-10 21:04:01]

On 2014/07/08 10:03:37, I haz the power (commit-bot) wrote:
> Change committed as 177654

For the record, the changes to base.py for the virtual test suite look good to
me.