Make window.innerWidth and innerHeight cause layout if needed.

Make window.innerWidth and innerHeight cause layout if needed.

innerWidth and innerHeight depend on the page scale. During page load the page
scale is set to the minimum. The minimum scale factor is based on the viewport
constraints and content width and so to get correct values during loading we
need to layout. This only happens on the main window and only if viewport tag
support is enabled, since otherwise the page loads at scale = 1.

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

[bokan, 2015-04-16 15:18:55]

bokan@chromium.org changed reviewers:
+ rune@opera.com

[bokan, 2015-04-16 15:18:56]

Rune, PTAL. WebViewImpl::updatePageDefinedViewportConstraints, called when a new
ViewportDescription is set, already marks the main frame as needing layout so we
just need to add the layout call to innerWidth/Height.

[rune, 2015-04-16 22:02:12]

https://codereview.chromium.org/1094673002/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/1094673002/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:951: // of the window.
The initial scale can be adjusted multiple times after layout, afaict, so I
think you can delete "before the first layout".

https://codereview.chromium.org/1094673002/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:987: 
Most code and comments in innerWidth and innerHeight are duplicated. How about
making a common method for returning the viewportSize and zoom-adjust the
correponding width/height value from it.

https://codereview.chromium.org/1094673002/diff/1/Source/web/tests/PinchViewp...
File Source/web/tests/PinchViewportTest.cpp (right):

https://codereview.chromium.org/1094673002/diff/1/Source/web/tests/PinchViewp...
Source/web/tests/PinchViewportTest.cpp:1721: // page will load with.
We really tests that the innerWidth/innerHeight makes sure the layout is
up-to-date. They actually trigger a full layout.

https://codereview.chromium.org/1094673002/diff/1/Source/web/tests/PinchViewp...
Source/web/tests/PinchViewportTest.cpp:1727: navigateTo(m_baseURL +
"window_dimensions.html");
Could you also add a test where the contents overflow the viewport to test that
we actually do layout, not just cause the viewport to be up-to-date?

[bokan, 2015-04-20 17:47:24]

Thanks Rune, new patch uploaded, ptal.

https://codereview.chromium.org/1094673002/diff/1/Source/core/frame/LocalDOMW...
File Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/1094673002/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:951: // of the window.
On 2015/04/16 22:02:12, rune wrote:
> The initial scale can be adjusted multiple times after layout, afaict, so I
> think you can delete "before the first layout".
> 

Done.

https://codereview.chromium.org/1094673002/diff/1/Source/core/frame/LocalDOMW...
Source/core/frame/LocalDOMWindow.cpp:987: 
On 2015/04/16 22:02:12, rune wrote:
> Most code and comments in innerWidth and innerHeight are duplicated. How about
> making a common method for returning the viewportSize and zoom-adjust the
> correponding width/height value from it.

Done.

https://codereview.chromium.org/1094673002/diff/1/Source/web/tests/PinchViewp...
File Source/web/tests/PinchViewportTest.cpp (right):

https://codereview.chromium.org/1094673002/diff/1/Source/web/tests/PinchViewp...
Source/web/tests/PinchViewportTest.cpp:1721: // page will load with.
On 2015/04/16 22:02:12, rune wrote:
> We really tests that the innerWidth/innerHeight makes sure the layout is
> up-to-date. They actually trigger a full layout.

Done.

https://codereview.chromium.org/1094673002/diff/1/Source/web/tests/PinchViewp...
Source/web/tests/PinchViewportTest.cpp:1727: navigateTo(m_baseURL +
"window_dimensions.html");
On 2015/04/16 22:02:12, rune wrote:
> Could you also add a test where the contents overflow the viewport to test
that
> we actually do layout, not just cause the viewport to be up-to-date?
> 

Done.

[rune, 2015-04-20 21:02:39]

lgtm

[bokan, 2015-04-21 13:10:26]

The CQ bit was checked by bokan@chromium.org

[bokan, 2015-04-21 13:10:26]

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/1094673002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2015-04-21 13:10:46]

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

[bokan, 2015-04-21 15:49:48]

The CQ bit was unchecked by bokan@chromium.org

[bokan, 2015-04-21 16:53:34]

The CQ bit was checked by bokan@chromium.org

[bokan, 2015-04-21 16:53:35]

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/1094673002/#ps60001
(title: "Fixed null check")

[commit-bot: I haz the power, 2015-04-21 16:54:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1094673002/60001

[commit-bot: I haz the power, 2015-04-21 18:08:44]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194150