Include scrollbar size in @media width/height

Include scrollbar size in @media width/height

Not including scrollbar size causes bad circular dependencies when
non-overlay scrollbars are created or removed.

CSS spec says we should include size of scroll bars when evaluate the
MQ rules about width/height.

BUG=236111
TEST=LayoutTests/fast/media/mq-size-include-scrollbars.html

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

[jamesr, 2013-05-06 22:52:03]

https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
File Source/core/platform/ScrollView.h (right):

https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
Source/core/platform/ScrollView.h:151: IntSize mediaSize() const;
is this the same as calling visibleContentRect(IncludeScrollbars).size() ? if
not, what's different?

[Xianzhu, 2013-05-06 22:55:44]

https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
File Source/core/platform/ScrollView.h (right):

https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
Source/core/platform/ScrollView.h:151: IntSize mediaSize() const;
On 2013/05/06 22:52:03, jamesr wrote:
> is this the same as calling visibleContentRect(IncludeScrollbars).size() ? if
> not, what's different?

They are the same for non-fixed-layout mode. For fixed-layout mode, it's the
fixed layout size which normally reflects the virtual viewport size.

[Xianzhu, 2013-05-06 22:59:09]

In addition, in fixed-layout mode when the page is scaled, mediaSize() is the
unscaledVisibleContentSize, so that the page layout won't be affected by the
page scale.

[Xianzhu, 2013-05-06 22:59:25]

#4 s/fixed-layout/non-fixed-layout/

[jamesr, 2013-05-06 23:38:49]

I have what is basically a naming nit but then this lgtm.

Can you also file a bug at bugs.webkit.org since they have the same bug?

https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
File Source/core/platform/ScrollView.h (right):

https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
Source/core/platform/ScrollView.h:151: IntSize mediaSize() const;
On 2013/05/06 22:55:44, Xianzhu wrote:
> On 2013/05/06 22:52:03, jamesr wrote:
> > is this the same as calling visibleContentRect(IncludeScrollbars).size() ?
if
> > not, what's different?
> 
> They are the same for non-fixed-layout mode. For fixed-layout mode, it's the
> fixed layout size which normally reflects the virtual viewport size.

Can we instead give layoutSize() the Includes/ExcludesScrollbars enums, using a
default value so you don't have to change callers. I'm a little concerned about
this being a near-duplicate of layoutSize() since the behavior of layoutSize()
isn't entirely obvious.  The "media" prefix isn't very descriptive either - it's
a concern of how this value is used (for media queries) but not how the value is
computed, and really platform/ScrollView shouldn't care about how media queries
work

[Xianzhu, 2013-05-07 00:24:02]

On 2013/05/06 23:38:49, jamesr wrote:
> I have what is basically a naming nit but then this lgtm.
> 
> Can you also file a bug at http://bugs.webkit.org since they have the same
bug?
> 
>
https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
> File Source/core/platform/ScrollView.h (right):
> 
>
https://codereview.chromium.org/14659008/diff/1/Source/core/platform/ScrollVi...
> Source/core/platform/ScrollView.h:151: IntSize mediaSize() const;
> On 2013/05/06 22:55:44, Xianzhu wrote:
> > On 2013/05/06 22:52:03, jamesr wrote:
> > > is this the same as calling visibleContentRect(IncludeScrollbars).size() ?
> if
> > > not, what's different?
> > 
> > They are the same for non-fixed-layout mode. For fixed-layout mode, it's the
> > fixed layout size which normally reflects the virtual viewport size.
> 
> Can we instead give layoutSize() the Includes/ExcludesScrollbars enums, using
a
> default value so you don't have to change callers. I'm a little concerned
about
> this being a near-duplicate of layoutSize() since the behavior of layoutSize()
> isn't entirely obvious.  The "media" prefix isn't very descriptive either -
it's
> a concern of how this value is used (for media queries) but not how the value
is
> computed, and really platform/ScrollView shouldn't care about how media
queries
> work

Agreed and done.

[commit-bot: I haz the power, 2013-05-07 00:35:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wangxianzhu@chromium.org/14659008/9001

[commit-bot: I haz the power, 2013-05-07 01:10:39]

Change committed as 149832