Created attachment 346101 [details]
screenshot of test case

There's a (very minimal...) unit test for scrolledwindow size requisition btw: https://git.gnome.org/browse/gtk+/tree/testsuite/gtk/scrolledwindow.c

Created attachment 346104 [details] [review]
ScrolledWindow: Don’t req size for auto scrollbars

(In reply to Timm Bäder from comment #2)
> There's a (very minimal...) unit test for scrolledwindow size requisition
> btw: https://git.gnome.org/browse/gtk+/tree/testsuite/gtk/scrolledwindow.c

Thanks. I patched it to also test permutations including overlay-scrolling == FALSE and policy == GTK_POLICY_AUTOMATIC. It fails on the first one!


The attached patch...

> POLICY_AUTOMATIC means scrollbars are only shown when needed, i.e. when
> the size of the window is not large enough to show the entire contents.
> So when returning the preferred size, we should exclude such scrollbars.
> 
> But measure() was bumping the preferred size if policy_may_be_visible()
> was TRUE for a given scrollbar, and that function returns TRUE for both
> POLICY_ALWAYS (good) and POLICY_AUTOMATIC (bad). So, too much size was
> requested/propagated for scrollbars with POLICY_AUTOMATIC. This had the
> effect that blank areas would appear where the scrollbars /would/ be if
> needed, but we have enough size for the child, so they don’t get shown.
> 
> Fix this by only requesting size for scrollbars that have POLICY_ALWAYS,
> rather than basing the decision on policy_may_be_visible().

...makes all 4 cases (overlay/fixed vs automatic/always) look correct to me.


It also means that newly written tests for fixed/automatic and overlay/always scollbars pass OK. Howeever, the suite fails as soon as it reaches the fixed/always case. Presumably, the calculation for the expected result presumably needs updated to accommodate the omnipresent non-overlay scrollbars in such a permutation.

typo:

(In reply to Daniel Boles from comment #3)
> (In reply to Timm Bäder from comment #2)
> > There's a (very minimal...) unit test for scrolledwindow size requisition
> > btw: https://git.gnome.org/browse/gtk+/tree/testsuite/gtk/scrolledwindow.c
> 
> Thanks. I patched it to also test permutations including overlay-scrolling
> == FALSE and policy == GTK_POLICY_AUTOMATIC. It fails on the first one!

s/AUTOMATIC/ALWAYS/


it initially failed on the min_content_width/fixed/automatic test:

> /sizing/scrolledwindow/min_content_width_overlay_automatic: OK
> /sizing/scrolledwindow/min_content_height_overlay_automatic: OK
> /sizing/scrolledwindow/max_content_width_overlay_automatic: OK
> /sizing/scrolledwindow/max_content_height_overlay_automatic: OK
> /sizing/scrolledwindow/min_max_content_width_overlay_automatic: OK
> /sizing/scrolledwindow/min_max_content_height_overlay_automatic: OK
> /sizing/scrolledwindow/min_content_width_fixed_automatic: **
> ERROR:/home/daniel/jhbuild/checkout/gnome/gtk+-3/testsuite/gtk/scrolledwindow.c:43:test_size:
> assertion failed (size == MIN_SIZE): (163 == 150)

but as mentioned, the patch to gtkscrolledwindow.c lets it get further:

> /sizing/scrolledwindow/min_content_width_overlay_automatic: OK
> /sizing/scrolledwindow/min_content_height_overlay_automatic: OK
> /sizing/scrolledwindow/max_content_width_overlay_automatic: OK
> /sizing/scrolledwindow/max_content_height_overlay_automatic: OK
> /sizing/scrolledwindow/min_max_content_width_overlay_automatic: OK
> /sizing/scrolledwindow/min_max_content_height_overlay_automatic: OK
> /sizing/scrolledwindow/min_content_width_fixed_automatic: OK
> /sizing/scrolledwindow/min_content_height_fixed_automatic: OK
> /sizing/scrolledwindow/max_content_width_fixed_automatic: OK
> /sizing/scrolledwindow/max_content_height_fixed_automatic: OK
> /sizing/scrolledwindow/min_max_content_width_fixed_automatic: OK
> /sizing/scrolledwindow/min_max_content_height_fixed_automatic: OK
> /sizing/scrolledwindow/min_content_width_overlay_always: OK
> /sizing/scrolledwindow/min_content_height_overlay_always: OK
> /sizing/scrolledwindow/max_content_width_overlay_always: OK
> /sizing/scrolledwindow/max_content_height_overlay_always: OK
> /sizing/scrolledwindow/min_max_content_width_overlay_always: OK
> /sizing/scrolledwindow/min_max_content_height_overlay_always: OK
> /sizing/scrolledwindow/min_content_width_fixed_always: **
> ERROR:/home/daniel/jhbuild/checkout/gnome/gtk+-3/testsuite/gtk/scrolledwindow.c:43:test_size:
> assertion failed (size == MIN_SIZE): (163 == 150)

I'll try to fix that up.


Anyway, I think the main patch makes sense, so a review would be appreciated.

Created attachment 346106 [details] [review]
testsuite/scrolledwindow—Test non-overlay/non-auto

It was only testing the default configuration where overlay-scrolling is
TRUE and the policy is POLICY_AUTOMATIC. We should also test FALSE and
POLICY_ALWAYS. This commit adds those tests and makes the !overlay &&
POLICY_ALWAYS case pass by excluding the size of the relevant scrollbar,
as we are only interested in whether the content size is as requested.

Attachment 346106 [details] pushed as 5a6e668 - testsuite/scrolledwindow—Test non-overlay/non-auto

This just makes sense, and refuses to break on any tests I could conjure up, so hopefully I'm right!

This causes criticals in e.g. the gtk3-demo Text View: Multiple Buffers demo:

> (gtk3-demo:7171): Gtk-CRITICAL **: gtk_box_gadget_distribute: assertion 'size >= 0' failed in GtkScrollbar

Apologies - I'll revert these commits and test harder next round.

Created attachment 346491 [details]
test case

non-stupid test case, i.e. without a requirement to recompile with different configurations of overlay/policy: it creates all 4 inside their own GtkWindows

Created attachment 346493 [details]
test case

removed erroneous return value that I had drafted but didn't end up using

Created attachment 346494 [details] [review]
ScrolledWindow—Don’t req size for auto-hidden bars

POLICY_AUTOMATIC means scrollbars are only shown when needed, i.e. when
the size of the window is not large enough to show the entire child. So
when measuring the preferred size, such scrollbars should be ignored.

But measure() requested size for *any* non-overlay bar. So we got space
for child plus bar, & having enough for the child meant POLICY_AUTOMATIC
hid the bar, leaving the extra space empty below/right of the child.

Fix this by only requesting size for non-overlay scrollbars if they are
set to POLICY_ALWAYS.

The new patch fixes the size request for the (!overlay && automatic) case, still passes the update to the reftest, and crucially isn't totally stupid like my last patch and therefore doesn't introduce any critical warnings.

The problem with the original patch was that, if either bar was POLICY_AUTOMATIC, I skipped the whole if block - and thus the bit that adds its request for the same orientation and the scrollable border. The real fix is to always do that, and only skip the inner if block, which adds its request for the perpendicular orientation.

a few questions arising from this:

https://git.gnome.org/browse/gtk+/tree/gtk/gtkscrolledwindow.c?h=gtk-3-22#n1885

- the bit that MAXes the ScrolledWindow's own width/height with that of the corresponding scrollbar seems like it also shouldn't be done unless the policy is ALWAYS

- what is the scrollable border actually used for? an example given in the GtkScrolledWindow doc is TreeView column headers. but in that case, shouldn't the border size always be added, even if the scrollbar policy can't be visible?

- should the section that gets said scrollable border from the child only be done if the child is non-null and visible? there is already an if for that just below

(In reply to Daniel Boles from comment #13)
> - what is the scrollable border actually used for? an example given in the
> GtkScrolledWindow doc is TreeView column headers. but in that case,
> shouldn't the border size always be added, even if the scrollbar policy
> can't be visible?

afaict it is just used for the overshoot blur:

> GTK+ can use this information to display overlayed graphics,
> like the overshoot indication, at the right position.

This is only applied if the scrollbar is visible - so if that's the only use of the border, this is fine.


> - should the section that gets said scrollable border from the child only be
> done if the child is non-null and visible? there is already an if for that
> just below

I suppose it doesn't matter in practice, because what use is a ScrolledWindow containing a hidden widget, but e.g. this means that in such a situation measure() always adds height for the header of said TreeView, even when it is hidden.

Another question: Are the checks for GTK_IS_SCROLLABLE(child) necessary? Because if the child being add()ed is not a GtkScrollable, then it gets wrapped within a GtkViewPort, which is. So it seems impossible to end up with a non-Scrollable child, making those checks unnecessary.

The documentation is also a bit self-contradictory for the same reason:

> If a widget has native scrolling abilities, it can be added to the
> GtkScrolledWindow with gtk_container_add(). If a widget does not, you must first
> add the widget to a GtkViewport, then add the GtkViewport to the scrolled
> window. gtk_container_add() will do this for you for widgets that don’t
> implement GtkScrollable natively, so you can ignore the presence of the
> viewport.

It basically says 'You must do this, but you do not need to do this!' It would be better to swap out "you must first" to "GTK+ will first".

Created attachment 346678 [details] [review]
[PATCH 1/3] ScrolledWindow: Clean up measure()

 • Only allocate variables & do calculations for the desired orientation
   – rather than measuring both and totally discarding the unwanted one
 • Move variables into the narrowest scopes where they are required

Created attachment 346679 [details] [review]
[PATCH 2/3] ScrolledWindow: Don’t req size for autohidden bars

POLICY_AUTOMATIC means scrollbars are only shown when needed, i.e. when
the size of the window is not large enough to show the entire child. So
when measuring the preferred size, such scrollbars should be ignored.

But measure() added size for *any* non-overlay scrollbar of the opposite
orientation, e.g. for horizontal size, it added the width of vscrollbar.
So we requested for child + bar, & having enough for child meant that the
policy hid the bar, leaving extra space empty below/right of the child.

Fix this by only adding size for such bars if they use POLICY_ALWAYS.

Created attachment 346680 [details] [review]
[PATCH 3/3] testsuite/scrolledwindow: Try non-overlay/non-auto

It was only testing the default configuration, where overlay scrolling
is on and both scrollbars use POLICY_AUTOMATIC. We should also test the
other 3 configurations introduced by non-overlay (here “fixed”)
scrollbars and/or those that use POLICY_ALWAYS.

Created attachment 346681 [details] [review]
ScrolledWindow: Don’t check if child is Scrollable

If the child added is not a Scrollable, it gets wrapped in a ViewPort –
which is. So it is impossible to end up with a non-Scrollable child.

Just check we have /any/ child where needed, which is semantically nicer

Created attachment 346682 [details] [review]
ScrolledWindow: Clarify doc of gtk_container_add()

It boiled down to ‘You must do this, but you do not need to do this’…
where ‘this’ is the requirement to wrap a non-Scrollable child in a
GtkViewPort first and then add() the latter to the ScrolledWindow.

In reality, our implementation of gtk_container_add() always does that
for the user, so they do not need to worry about that detail. It is
worth mentioning, but avoid making it sound like their responsibility.

Created attachment 346758 [details] [review]
[PATCH 1/3] ScrolledWindow: Optimise/cleanup measure()

• Only calculate the specified dimension – rather than measuring both &
   discarding the other (it will often be calculated again right after)
 • Only measure a given child scrollbar if it may be visible, not always
 • Move variables into narrowest scopes & otherwise improve readability

Please could someone review the remaining patches? I'm very confident in them this time around. We have an obvious bugfix, better unit test coverage, significant improvements to readability and performance of measure(), fewer unneeded typecasts, and better docs. It'd be nice to get these into 3.22.10 and 3.89.

Review of attachment 346682 [details] [review]:

::: gtk/gtkscrolledwindow.c
@@ +75,3 @@
+ * is, it is added directly. If it is not, it must be added to a #GtkViewport,
+ * before adding the #GtkViewport to the ScrolledWindow, and gtk_container_add()
+ * will do this for you. Therefore, you can ignore the need for the viewport.

I don't like the "if it isn't, it must be added..." part here. People would probably commonly just stop reading after that sentence and then add their widget to a GtkViewport manually. Just using "if it isn't, #GtkScrolledWindow will add the given widget to a #GtkViewport first" or something similar (I'm not sure I like that sentence either) would be better IMO.

Thanks for the review. I did realise earlier when skimming these patches that I don't like that wording much, or at least the order of it. I'll fix it up later.

Review of attachment 346758 [details] [review]:

Dunno about the overall size requisition changes. From the git log they make sense of course and I like the result better but there are too many cases here for me to have an overview; If they don't break anything it's ok.

::: gtk/gtkscrolledwindow.c
@@ +1706,3 @@
   GtkScrolledWindow *scrolled_window = GTK_SCROLLED_WINDOW (widget);
   GtkScrolledWindowPrivate *priv = scrolled_window->priv;
+  gint minimum_req = 0, natural_req = 0;

I heavily prefer int instead of gint in newly written code.

@@ +1718,3 @@
   if (child && gtk_widget_get_visible (child))
     {
+      gint min_child_size, nat_child_size;

Same here.

@@ +1726,3 @@
       if (orientation == GTK_ORIENTATION_HORIZONTAL)
+        {
+          if (priv->propagate_natural_width)

No comment to all the whitespace changes, I like them but other might disagree :)

Review of attachment 346681 [details] [review]:

Yeah that makes sense IMO.

I actually can't apply attachment 346681 [details] [review] because...

Applying: ScrolledWindow: Don’t check if child is Scrollable
error: sha1 information is lacking or useless (gtk/gtkscrolledwindow.c).
error: could not build fake ancestor
Patch failed at 0001 ScrolledWindow: Don’t check if child is Scrollable
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/ScrolledWindow-Dont-check-if-child-is-Scrollable-eEfWxb.patch


Not sure what's up with that, I'm fairly certain I'm using the correct order this time.

(In reply to Timm Bäder from comment #27)
> I actually can't apply attachment 346681 [details] [review] [review] because...
> 
> Applying: ScrolledWindow: Don’t check if child is Scrollable
> error: sha1 information is lacking or useless (gtk/gtkscrolledwindow.c).
> error: could not build fake ancestor
>
> [...]
> 
> Not sure what's up with that, I'm fairly certain I'm using the correct order
> this time.

Weird, I don't quite understand that either. I'll rebase tonight.

Thanks for the heads-up, and the other reviews!

Hi folks - CCing you in because you worked on Bug 766569, and I hope you can perhaps confirm whether Attachment 346679 [details] is as logical as I think it is. :)

Created attachment 348223 [details] [review]
ScrolledWindow: Don’t check if child is Scrollable

If the child added is not a Scrollable, it gets wrapped in a ViewPort –
which is. So it is impossible to end up with a non-Scrollable child.

Just check we have /any/ child where needed, which is semantically nicer

—

Uploading a rebase of these patches. Timm reviewed this, so I’ll probably commit
it soon.

Created attachment 348224 [details] [review]
ScrolledWindow: Clarify doc of gtk_container_add()

It boiled down to ‘You must do this, but you do not need to do this’…
where ‘this’ is the requirement to wrap a non-Scrollable child in a
GtkViewPort first and then add() the latter to the ScrolledWindow.

In reality, our implementation of gtk_container_add() always does that
for the user, so they do not need to worry about that detail. It is
worth mentioning, but avoid making it sound like their responsibility.

—

Updated as per Timm's comments. This makes it clear that the user doesn't need
to worry about GtkViewport. By doing so, the doc is simpler and easier to read.

Created attachment 348225 [details] [review]
ScrolledWindow: Optimise and clean up measure()

• Only calculate the specified dimension – rather than measuring both &
   discarding the other (which will often be recalculated right after)
 • Only measure a given child scrollbar if it may be visible, not always
 • Move variables into narrowest scopes & otherwise improve readability

—

Updated as per Timm's review - dropping the incidental whitespace changes, just
in case.

Created attachment 348226 [details] [review]
ScrolledWindow: Don’t req size for autohidden bars

POLICY_AUTOMATIC means scrollbars are only shown when needed, i.e. when
the size of the window is not large enough to show the entire child. So
when measuring the preferred size, such scrollbars should be ignored.

But measure() added size for *any* non-overlay scrollbar of the opposite
orientation, e.g. for horizontal size, it added the width of vscrollbar.
So we requested for child + bar, & having enough for child meant that the
policy hid the bar, leaving extra space empty below/right of the child.

Fix this by only adding size for such bars if they use POLICY_ALWAYS.

—

This is the main point of this BZ and hence the patch I really need feedback on.
It seems so simple that it must be either totally right or totally wrong. The
testsuite still passes (albeit after the next commit where I make it pass...),
and every app I've tried is A-OK, but I won't claim to know much about sizing.

So I'd appreciate an informed review of this, letting me know which way to go
next, one way or another.

Created attachment 348227 [details] [review]
testsuite/scrolledwindow: Try non-overlay/non-auto

It was only testing the default configuration, where overlay scrolling
is on and both scrollbars use POLICY_AUTOMATIC. We should also test the
other 3 configurations introduced by non-overlay (here “fixed”)
scrollbars and/or those that use POLICY_ALWAYS.

—

As previously explained, this adds handling for other cases that were not
covered originally, and it also fixes these to pass the previous patch that
doesn't superfluously request size for automatically hidden scrollbars.

Review of attachment 348227 [details] [review]:

::: gtk/gtkscrolledwindow.c
@@ +57,3 @@
  * @See_also: #GtkScrollable, #GtkViewport, #GtkAdjustment
  *
+ * GtkScrolledWindow is a container that accepts a single child widget, makes

Ugh, that's what happens when I rebase a bunch of times. This line needs to be squashed into the commit that edits the docs, so just pretend it is, I guess.

Review of attachment 348223 [details] [review]:

::: gtk/gtkscrolledwindow.c
@@ +1464,3 @@
                        : GTK_SCROLL_MINIMUM;
+      vscroll_policy = scrollable_child
+                       ? gtk_scrollable_get_vscroll_policy (scrollable_chiild)

typo here: chiild

Created attachment 348345 [details] [review]
ScrolledWindow: Don’t check if child is Scrollable

If the child added is not a Scrollable, it gets wrapped in a ViewPort –
which is. So it is impossible to end up with a non-Scrollable child.

Just check we have /any/ child where needed, which is semantically nicer

Created attachment 348346 [details] [review]
ScrolledWindow: Clarify doc of gtk_container_add()

It boiled down to ‘You must do this, but you do not need to do this’…
where ‘this’ is the requirement to wrap a non-Scrollable child in a
GtkViewPort first and then add() the latter to the ScrolledWindow.

In reality, our implementation of gtk_container_add() always does that
for the user, so they do not need to worry about that detail. It is
worth mentioning, but avoid making it sound like their responsibility.

Created attachment 348347 [details] [review]
ScrolledWindow: Optimise and clean up measure()

• Only calculate the specified dimension – rather than measuring both &
   discarding the other (which will often be recalculated right after)
 • Only measure a given child scrollbar if it may be visible, not always
 • Move variables into narrowest scopes & otherwise improve readability

Created attachment 348348 [details] [review]
ScrolledWindow: Don’t req size for autohidden bars

POLICY_AUTOMATIC means scrollbars are only shown when needed, i.e. when
the size of the window is not large enough to show the entire child. So
when measuring the preferred size, such scrollbars should be ignored.

But measure() added size for *any* non-overlay scrollbar of the opposite
orientation, e.g. for horizontal size, it added the width of vscrollbar.
So we requested for child + bar, & having enough for child meant that the
policy hid the bar, leaving extra space empty below/right of the child.

Fix this by only adding size for such bars if they use POLICY_ALWAYS.

Created attachment 348349 [details] [review]
testsuite/scrolledwindow: Try non-overlay/non-auto

It was only testing the default configuration, where overlay scrolling
is on and both scrollbars use POLICY_AUTOMATIC. We should also test the
other 3 configurations introduced by non-overlay (here “fixed”)
scrollbars and/or those that use POLICY_ALWAYS.

Comment on attachment 348346 [details] [review]
ScrolledWindow: Clarify doc of gtk_container_add()

committed along with a bunch of other tweaks to the intro docs:
https://git.gnome.org/browse/gtk+/commit/?id=52547054a55a7ddd4c0f62c170e37d77e0b09c7a

Has anyone had any thoughts on the remaining patches yet? IIRC Timm suggested just pushing to master, which should shake out any side-effects, though I can't imagine any drawbacks to this (which, of course, may just point to me being unimaginative)

The excessive request for automatically hidden, non-overlay scrollbars requires me to avoid using the sensible code and do a lot of working around the problem by measuring the child myself. I'm quite surprised it doesn't seem to be aggravating anyone else; I guess almost nobody uses this configuration of scrollbar, somehow.

Debarshi also suggested pushing to master and seeing how we get on, so I have pushed updated versions of the remaining patches there:

https://git.gnome.org/browse/gtk+/commit/?id=1a95c259d7b37f11668f60b276100e27e2ed3c18
https://git.gnome.org/browse/gtk+/commit/?id=a96c5864509b175d1120e467774343fad8d96769
https://git.gnome.org/browse/gtk+/commit/?id=9546673d3382602d79d990c0cdbd30a5112fb80d

No doubt there are other gremlins still lurking in these files - but it is my hope that I'm only removing one and not adding any!

Will keep this open to see how we get on with these in master, and hopefully get them into 3.22 if they prove to work OK. Of course, any feedback to that end will be appreciated.

pushed to gtk-3-22 minus the refactoring, closing optimistically

commit 2aa4248e8f08d213049b2e0ab8aa879a0ca17e36
Author: Daniel Boles <dboles@src.gnome.org>
Date:   Fri Feb 24 22:46:05 2017 +0000

    testsuite/scrolledwindow: Try non-overlay/non-auto
    
    It was only testing the default configuration, where overlay scrolling
    is on and both scrollbars use POLICY_AUTOMATIC. We should also test the
    other 3 configurations that are available by including non-overlay
    scrollbars and/or those that use POLICY_ALWAYS.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778853

 testsuite/gtk/scrolledwindow.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 187 insertions(+), 32 deletions(-)

commit fcfad2dd7eaa881efa038ad6448990bba98b8fcd
Author: Daniel Boles <dboles@src.gnome.org>
Date:   Fri Feb 24 22:46:05 2017 +0000

    ScrolledWindow: Don’t req size for autohidden bars
    
    POLICY_AUTOMATIC means scrollbars are only shown when needed, i.e. when
    the size of the window is not large enough to show the entire child. So
    when measuring the preferred size, such scrollbars should be ignored.
    
    But measure() added size for *any* non-overlay scrollbar of the opposite
    orientation, e.g. for horizontal size, it added the width of vscrollbar.
    So we requested for child + bar, & having enough for child meant that the
    policy hid the bar, leaving extra space empty below/right of the child.
    
    Fix this by only adding size for such bars if they use POLICY_ALWAYS.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=778853

 gtk/gtkscrolledwindow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

*** Bug 712220 has been marked as a duplicate of this bug. ***

Comment on attachment 348349 [details] [review]
testsuite/scrolledwindow: Try non-overlay/non-auto

>From 6feafe263b839d0cda21cb2eb28be91dc64bf054 Mon Sep 17 00:00:00 2001
>From: Daniel Boles <dboles@src.gnome.org>
>Date: Fri, 24 Feb 2017 22:46:05 +0000
>Subject: [PATCH] testsuite/scrolledwindow: Try non-overlay/non-auto
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>It was only testing the default configuration, where overlay scrolling
>is on and both scrollbars use POLICY_AUTOMATIC. We should also test the
>other 3 configurations introduced by non-overlay (here âfixedâ)
>scrollbars and/or those that use POLICY_ALWAYS.
>
>https://bugzilla.gnome.org/show_bug.cgi?id=778853
>---
> testsuite/gtk/scrolledwindow.c | 217 +++++++++++++++++++++++++++++++++++------
> 1 file changed, 186 insertions(+), 31 deletions(-)
>
>diff --git a/testsuite/gtk/scrolledwindow.c b/testsuite/gtk/scrolledwindow.c
>index a54e85cb15..7dba662494 100644
>--- a/testsuite/gtk/scrolledwindow.c
>+++ b/testsuite/gtk/scrolledwindow.c
>@@ -12,10 +12,13 @@ typedef enum
> 
> static void
> test_size (GtkOrientation orientation,
>+           gboolean       overlay,
>+           GtkPolicyType  policy,
>            TestProperty   prop)
> {
>   GtkWidget *scrolledwindow, *box;
>-  int size, child_size;
>+  int min_size, max_size, child_size;
>+  int scrollbar_size = 0;
> 
>   box = gtk_box_new (GTK_ORIENTATION_VERTICAL, 0);
>   gtk_widget_set_hexpand (box, TRUE);
>@@ -24,6 +27,8 @@ test_size (GtkOrientation orientation,
>   scrolledwindow = gtk_scrolled_window_new (NULL, NULL);
>   gtk_scrolled_window_set_propagate_natural_width (GTK_SCROLLED_WINDOW (scrolledwindow), TRUE);
>   gtk_scrolled_window_set_propagate_natural_height (GTK_SCROLLED_WINDOW (scrolledwindow), TRUE);
>+  gtk_scrolled_window_set_overlay_scrolling (GTK_SCROLLED_WINDOW (scrolledwindow), overlay);
>+  gtk_scrolled_window_set_policy (GTK_SCROLLED_WINDOW (scrolledwindow), policy, policy);
>   gtk_container_add (GTK_CONTAINER (scrolledwindow), box);
> 
>   /* Testing the content-width property */
>@@ -33,9 +38,7 @@ test_size (GtkOrientation orientation,
>         {
>           gtk_scrolled_window_set_min_content_width (GTK_SCROLLED_WINDOW (scrolledwindow), MIN_SIZE);
> 
>-          gtk_widget_get_preferred_width (scrolledwindow, &size, NULL);
>-
>-          g_assert_cmpint (size, ==, MIN_SIZE);
>+          gtk_widget_get_preferred_width (scrolledwindow, &min_size, NULL);
>         }
> 
>       if (prop & MAXIMUM_CONTENT)
>@@ -47,11 +50,17 @@ test_size (GtkOrientation orientation,
>            * Here, the content is purposely bigger than the scrolled window,
>            * so it should grow up to max-content-width.
>            */
>-          gtk_widget_get_preferred_width (scrolledwindow, NULL, &size);
>+          gtk_widget_get_preferred_width (scrolledwindow, NULL, &max_size);
>           gtk_widget_get_preferred_width (box, &child_size, NULL);
>+        }
> 
>-          g_assert_cmpint (child_size, ==, BOX_SIZE);
>-          g_assert_cmpint (size, ==, MAX_SIZE);
>+      /* If the relevant scrollbar is non-overlay and always shown, it is added
>+       * to the preferred size. When comparing to the expected size, we need to
>+       * to exclude that extra, as we are only interested in the content size */
>+      if (!overlay && policy == GTK_POLICY_ALWAYS)
>+        {
>+          GtkWidget *scrollbar = gtk_scrolled_window_get_vscrollbar (GTK_SCROLLED_WINDOW (scrolledwindow));
>+          gtk_widget_get_preferred_width (scrollbar, &scrollbar_size, NULL);
>         }
>     }
>   /* Testing the content-height property */
>@@ -61,9 +70,7 @@ test_size (GtkOrientation orientation,
>         {
>           gtk_scrolled_window_set_min_content_height (GTK_SCROLLED_WINDOW (scrolledwindow), MIN_SIZE);
> 
>-          gtk_widget_get_preferred_height (scrolledwindow, &size, NULL);
>-
>-          g_assert_cmpint (size, ==, MIN_SIZE);
>+          gtk_widget_get_preferred_height (scrolledwindow, &min_size, NULL);
>         }
> 
>       if (prop & MAXIMUM_CONTENT)
>@@ -75,66 +82,214 @@ test_size (GtkOrientation orientation,
>            * Here, the content is purposely bigger than the scrolled window,
>            * so it should grow up to max-content-height.
>            */
>-          gtk_widget_get_preferred_height (scrolledwindow, NULL, &size);
>+          gtk_widget_get_preferred_height (scrolledwindow, NULL, &max_size);
>           gtk_widget_get_preferred_height (box, &child_size, NULL);
>+        }
> 
>-          g_assert_cmpint (child_size, ==, BOX_SIZE);
>-          g_assert_cmpint (size, ==, MAX_SIZE);
>+      if (!overlay && policy == GTK_POLICY_ALWAYS)
>+        {
>+          GtkWidget *scrollbar = gtk_scrolled_window_get_hscrollbar (GTK_SCROLLED_WINDOW (scrolledwindow));
>+          gtk_widget_get_preferred_height (scrollbar, &scrollbar_size, NULL);
>         }
>     }
>+
>+  if (prop & MINIMUM_CONTENT)
>+    {
>+      min_size -= scrollbar_size;
>+      g_assert_cmpint (min_size, ==, MIN_SIZE);
>+    }
>+
>+  if (prop & MAXIMUM_CONTENT)
>+    {
>+      g_assert_cmpint (child_size, ==, BOX_SIZE);
>+
>+      max_size -= scrollbar_size;
>+      g_assert_cmpint (max_size, ==, MAX_SIZE);
>+    }
>+}
>+
>+
>+static void
>+min_content_width_overlay_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, TRUE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT);
>+}
>+
>+static void
>+min_content_height_overlay_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, TRUE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT);
>+}
>+
>+static void
>+max_content_width_overlay_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, TRUE, GTK_POLICY_AUTOMATIC, MAXIMUM_CONTENT);
>+}
>+
>+static void
>+max_content_height_overlay_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, TRUE, GTK_POLICY_AUTOMATIC, MAXIMUM_CONTENT);
>+}
>+
>+static void
>+min_max_content_width_overlay_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, TRUE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+}
>+
>+static void
>+min_max_content_height_overlay_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, TRUE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+}
>+
>+
>+static void
>+min_content_width_fixed_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, FALSE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT);
>+}
>+
>+static void
>+min_content_height_fixed_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, FALSE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT);
>+}
>+
>+static void
>+max_content_width_fixed_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, FALSE, GTK_POLICY_AUTOMATIC, MAXIMUM_CONTENT);
> }
> 
>+static void
>+max_content_height_fixed_automatic (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, FALSE, GTK_POLICY_AUTOMATIC, MAXIMUM_CONTENT);
>+}
> 
> static void
>-min_content_width (void)
>+min_max_content_width_fixed_automatic (void)
> {
>-  test_size (GTK_ORIENTATION_HORIZONTAL, MINIMUM_CONTENT);
>+  test_size (GTK_ORIENTATION_HORIZONTAL, FALSE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT | MAXIMUM_CONTENT);
> }
> 
> static void
>-min_content_height (void)
>+min_max_content_height_fixed_automatic (void)
> {
>-  test_size (GTK_ORIENTATION_VERTICAL, MINIMUM_CONTENT);
>+  test_size (GTK_ORIENTATION_VERTICAL, FALSE, GTK_POLICY_AUTOMATIC, MINIMUM_CONTENT | MAXIMUM_CONTENT);
> }
> 
>+
> static void
>-max_content_width (void)
>+min_content_width_overlay_always (void)
> {
>-  test_size (GTK_ORIENTATION_HORIZONTAL, MAXIMUM_CONTENT);
>+  test_size (GTK_ORIENTATION_HORIZONTAL, TRUE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT);
> }
> 
> static void
>-max_content_height (void)
>+min_content_height_overlay_always (void)
> {
>-  test_size (GTK_ORIENTATION_VERTICAL, MAXIMUM_CONTENT);
>+  test_size (GTK_ORIENTATION_VERTICAL, TRUE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT);
> }
> 
> static void
>-min_max_content_width (void)
>+max_content_width_overlay_always (void)
> {
>-  test_size (GTK_ORIENTATION_HORIZONTAL, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+  test_size (GTK_ORIENTATION_HORIZONTAL, TRUE, GTK_POLICY_ALWAYS, MAXIMUM_CONTENT);
> }
> 
> static void
>-min_max_content_height (void)
>+max_content_height_overlay_always (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, TRUE, GTK_POLICY_ALWAYS, MAXIMUM_CONTENT);
>+}
>+
>+static void
>+min_max_content_width_overlay_always (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, TRUE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+}
>+
>+static void
>+min_max_content_height_overlay_always (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, TRUE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+}
>+
>+
>+static void
>+min_content_width_fixed_always (void)
> {
>-  test_size (GTK_ORIENTATION_VERTICAL, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+  test_size (GTK_ORIENTATION_HORIZONTAL, FALSE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT);
> }
> 
>+static void
>+min_content_height_fixed_always (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, FALSE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT);
>+}
>+
>+static void
>+max_content_width_fixed_always (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, FALSE, GTK_POLICY_ALWAYS, MAXIMUM_CONTENT);
>+}
>+
>+static void
>+max_content_height_fixed_always (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, FALSE, GTK_POLICY_ALWAYS, MAXIMUM_CONTENT);
>+}
>+
>+static void
>+min_max_content_width_fixed_always (void)
>+{
>+  test_size (GTK_ORIENTATION_HORIZONTAL, FALSE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+}
>+
>+static void
>+min_max_content_height_fixed_always (void)
>+{
>+  test_size (GTK_ORIENTATION_VERTICAL, FALSE, GTK_POLICY_ALWAYS, MINIMUM_CONTENT | MAXIMUM_CONTENT);
>+}
>+
>+
> int
> main (int argc, char **argv)
> {
>   gtk_init ();
>   g_test_init (&argc, &argv, NULL);
> 
>-  g_test_add_func ("/sizing/scrolledwindow/min_content_width", min_content_width);
>-  g_test_add_func ("/sizing/scrolledwindow/min_content_height", min_content_height);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_width_overlay_automatic", min_content_width_overlay_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_height_overlay_automatic", min_content_height_overlay_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_width_overlay_automatic", max_content_width_overlay_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_height_overlay_automatic", max_content_height_overlay_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_width_overlay_automatic", min_max_content_width_overlay_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_height_overlay_automatic", min_max_content_height_overlay_automatic);
>+
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_width_fixed_automatic", min_content_width_fixed_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_height_fixed_automatic", min_content_height_fixed_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_width_fixed_automatic", max_content_width_fixed_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_height_fixed_automatic", max_content_height_fixed_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_width_fixed_automatic", min_max_content_width_fixed_automatic);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_height_fixed_automatic", min_max_content_height_fixed_automatic);
> 
>-  g_test_add_func ("/sizing/scrolledwindow/max_content_width", max_content_width);
>-  g_test_add_func ("/sizing/scrolledwindow/max_content_height", max_content_height);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_width_overlay_always", min_content_width_overlay_always);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_height_overlay_always", min_content_height_overlay_always);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_width_overlay_always", max_content_width_overlay_always);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_height_overlay_always", max_content_height_overlay_always);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_width_overlay_always", min_max_content_width_overlay_always);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_height_overlay_always", min_max_content_height_overlay_always);
> 
>-  g_test_add_func ("/sizing/scrolledwindow/min_max_content_width", min_max_content_width);
>-  g_test_add_func ("/sizing/scrolledwindow/min_max_content_height", min_max_content_height);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_width_fixed_always", min_content_width_fixed_always);
>+  g_test_add_func ("/sizing/scrolledwindow/min_content_height_fixed_always", min_content_height_fixed_always);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_width_fixed_always", max_content_width_fixed_always);
>+  g_test_add_func ("/sizing/scrolledwindow/max_content_height_fixed_always", max_content_height_fixed_always);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_width_fixed_always", min_max_content_width_fixed_always);
>+  g_test_add_func ("/sizing/scrolledwindow/min_max_content_height_fixed_always", min_max_content_height_fixed_always);
> 
>   return g_test_run ();
> }
>-- 
>2.11.0