PrintContext::pageProperty() shouldn't use 0 as page height.

PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by window.internals,
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

Also no need to explicitly lay out the document here, since
PrintContext::begin() does it for us.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org

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

[mstensho (USE GERRIT), 2015-12-09 20:30:53]

Look! I found something for you today after all! :)

Not sure if this change is worthwhile. I might as well simply remove the TODO in
LayoutView instead (and keep the sanity check).

PrintContext::pageProperty() looks like half-finished testing code.

[rune, 2015-12-09 23:56:38]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2015-12-09 23:56:39]

https://codereview.chromium.org/1510353002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/printing/pageProperty-with-multicol.html
(right):

https://codereview.chromium.org/1510353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/printing/pageProperty-with-multicol.html:12: if
(window.testRunner) {
You should check for window.internals. With --expose-internals-for-testing you
can do the test below without the testRunner.

[mstensho (USE GERRIT), 2015-12-10 09:07:32]

https://codereview.chromium.org/1510353002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/printing/pageProperty-with-multicol.html
(right):

https://codereview.chromium.org/1510353002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/printing/pageProperty-with-multicol.html:12: if
(window.testRunner) {
On 2015/12/09 23:56:38, rune wrote:
> You should check for window.internals. With --expose-internals-for-testing you
> can do the test below without the testRunner.

Done.

So much for copy-pasting from a decade old test with brain turned off. :)

[rune, 2015-12-10 09:44:50]

lgtm

https://codereview.chromium.org/1510353002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/PrintContext.cpp (left):

https://codereview.chromium.org/1510353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/PrintContext.cpp:263:
document->updateLayout();
printContext.begin() will update the style/layout. This call can be removed.

[mstensho (USE GERRIT), 2015-12-10 09:45:58]

https://codereview.chromium.org/1510353002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/PrintContext.cpp (left):

https://codereview.chromium.org/1510353002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/PrintContext.cpp:263:
document->updateLayout();
On 2015/12/10 09:44:50, rune wrote:
> printContext.begin() will update the style/layout. This call can be removed.

Done.

[mstensho (USE GERRIT), 2015-12-10 09:47:05]

Description was changed from

==========
PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by testRunner (via window.internals),
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org
==========

to

==========
PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by window.internals,
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

Also no need to explicitly lay out the document here, since
PrintContext::begin() does it for us.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org
==========

[mstensho (USE GERRIT), 2015-12-10 09:47:16]

The CQ bit was checked by mstensho@opera.com

[mstensho (USE GERRIT), 2015-12-10 09:47:16]

The patchset sent to the CQ was uploaded after l-g-t-m from rune@opera.com
Link to the patchset: https://codereview.chromium.org/1510353002/#ps40001
(title: "No need to call updateLayout() manually. PrintContext::begin() does it
for us.")

[commit-bot: I haz the power, 2015-12-10 09:47:34]

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

[commit-bot: I haz the power, 2015-12-10 11:52:09]

Description was changed from

==========
PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by window.internals,
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

Also no need to explicitly lay out the document here, since
PrintContext::begin() does it for us.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org
==========

to

==========
PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by window.internals,
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

Also no need to explicitly lay out the document here, since
PrintContext::begin() does it for us.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org
==========

[commit-bot: I haz the power, 2015-12-10 11:52:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-10 11:53:53]

Description was changed from

==========
PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by window.internals,
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

Also no need to explicitly lay out the document here, since
PrintContext::begin() does it for us.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org
==========

to

==========
PrintContext::pageProperty() shouldn't use 0 as page height.

This function only seems to be used by window.internals,
although WebFrame also provides a method that ends up there. It's only
partially implemented, though. It recognizes "margin-left", but not
"margin-right", for instance. Anyway, use a better page height than 0.

Also no need to explicitly lay out the document here, since
PrintContext::begin() does it for us.

With this change, we no longer need to perform a page height sanity check in
LayoutView::layout() before creating a ViewFragmentationContext.

R=leviw@chromium.org

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

[commit-bot: I haz the power, 2015-12-10 11:53:54]

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

[leviw_travelin_and_unemployed, 2015-12-10 19:38:07]

On 2015/12/10 at 09:44:50, rune wrote:
> lgtm
> 
>
https://codereview.chromium.org/1510353002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/page/PrintContext.cpp (left):
> 
>
https://codereview.chromium.org/1510353002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/page/PrintContext.cpp:263:
document->updateLayout();
> printContext.begin() will update the style/layout. This call can be removed.

Thanks for the assist :D