Gtk content shell: make the window shrinkable.

Gtk content shell: make the window shrinkable.

Don't treat a window resize request by setting the content widget's
minimum size; instead just resize the top level window.

R=
BUG=236301
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=201315

[jochen (gone - plz use gerrit), 2013-05-14 07:25:32]

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc
File content/shell/shell_gtk.cc (right):

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc#ne...
content/shell/shell_gtk.cc:215: gtk_window_resize(window_, width, height);
this will break headless mode where window_ is always NULL

[mstensho (USE GERRIT), 2013-05-14 09:28:23]

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc
File content/shell/shell_gtk.cc (right):

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc#ne...
content/shell/shell_gtk.cc:215: gtk_window_resize(window_, width, height);
On 2013/05/14 07:25:33, jochen wrote:
> this will break headless mode where window_ is always NULL

So would the right thing be to keep the old code for headless?

I.e. "else if (web_contents_) blahblah..." ?

[jochen (gone - plz use gerrit), 2013-05-14 09:31:39]

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc
File content/shell/shell_gtk.cc (right):

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc#ne...
content/shell/shell_gtk.cc:215: gtk_window_resize(window_, width, height);
On 2013/05/14 09:28:24, mstensho wrote:
> On 2013/05/14 07:25:33, jochen wrote:
> > this will break headless mode where window_ is always NULL
> 
> So would the right thing be to keep the old code for headless?
> 
> I.e. "else if (web_contents_) blahblah..." ?

Not sure. We always want the native view to be the specified size. So if window_
!= NULL, you need to add the size for the url bar etc..

[mstensho (USE GERRIT), 2013-05-14 09:40:16]

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc
File content/shell/shell_gtk.cc (right):

https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc#ne...
content/shell/shell_gtk.cc:215: gtk_window_resize(window_, width, height);
On 2013/05/14 09:31:39, jochen wrote:
> On 2013/05/14 09:28:24, mstensho wrote:
> > On 2013/05/14 07:25:33, jochen wrote:
> > > this will break headless mode where window_ is always NULL
> > 
> > So would the right thing be to keep the old code for headless?
> > 
> > I.e. "else if (web_contents_) blahblah..." ?
> 
> Not sure. We always want the native view to be the specified size. So if
window_
> != NULL, you need to add the size for the url bar etc..

I'm sorry, I don't understand. Can you provide me with a test case for
non-headless mode where my change causes problems?

[jochen (gone - plz use gerrit), 2013-05-14 09:46:19]

On 2013/05/14 09:40:16, mstensho wrote:
> https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc
> File content/shell/shell_gtk.cc (right):
> 
>
https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc#ne...
> content/shell/shell_gtk.cc:215: gtk_window_resize(window_, width, height);
> On 2013/05/14 09:31:39, jochen wrote:
> > On 2013/05/14 09:28:24, mstensho wrote:
> > > On 2013/05/14 07:25:33, jochen wrote:
> > > > this will break headless mode where window_ is always NULL
> > > 
> > > So would the right thing be to keep the old code for headless?
> > > 
> > > I.e. "else if (web_contents_) blahblah..." ?
> > 
> > Not sure. We always want the native view to be the specified size. So if
> window_
> > != NULL, you need to add the size for the url bar etc..
> 
> I'm sorry, I don't understand. Can you provide me with a test case for
> non-headless mode where my change causes problems?

Sure, e.g. with your change

run-webkit-tests --content-shell --debug --additional-drt-flag
--disable-headless-for-layout-tests  svg/W3C-SVG-1.1-SE/color-prop-05-t.svg

fails. Note that this is the not-headless case. In the headless case, we also
set the size from the renderer (e.g. on Mac, there's no SizeTo method on the
browser side), so the test passes on headless.

[mstensho (USE GERRIT), 2013-05-14 11:11:30]

On 2013/05/14 09:46:19, jochen wrote:
> On 2013/05/14 09:40:16, mstensho wrote:
> > https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc
> > File content/shell/shell_gtk.cc (right):
> > 
> >
>
https://codereview.chromium.org/14496004/diff/1/content/shell/shell_gtk.cc#ne...
> > content/shell/shell_gtk.cc:215: gtk_window_resize(window_, width, height);
> > On 2013/05/14 09:31:39, jochen wrote:
> > > On 2013/05/14 09:28:24, mstensho wrote:
> > > > On 2013/05/14 07:25:33, jochen wrote:
> > > > > this will break headless mode where window_ is always NULL
> > > > 
> > > > So would the right thing be to keep the old code for headless?
> > > > 
> > > > I.e. "else if (web_contents_) blahblah..." ?
> > > 
> > > Not sure. We always want the native view to be the specified size. So if
> > window_
> > > != NULL, you need to add the size for the url bar etc..
> > 
> > I'm sorry, I don't understand. Can you provide me with a test case for
> > non-headless mode where my change causes problems?
> 
> Sure, e.g. with your change
> 
> run-webkit-tests --content-shell --debug --additional-drt-flag
> --disable-headless-for-layout-tests  svg/W3C-SVG-1.1-SE/color-prop-05-t.svg
> 
> fails. Note that this is the not-headless case. In the headless case, we also
> set the size from the renderer (e.g. on Mac, there's no SizeTo method on the
> browser side), so the test passes on headless.

This test also fails WITHOUT my change.

[jochen (gone - plz use gerrit), 2013-05-14 11:19:40]

hum, for me (and on the bots) it passes. Can you make sure you recompiled the
correct mode without your patch (i.e. if you pass --debug, you need to recompile
Debug)

[mstensho (USE GERRIT), 2013-05-14 11:32:19]

I tried both release and debug and rebuilt with target all_webkit and
content_shell.

Here's what I do for release:

$ ninja -C out/Release content_shell all_webkit
$ ./third_party/WebKit/Tools/Scripts/run-webkit-tests  --content-shell
--additional-drt-flag --disable-headless-for-layout-tests 
svg/W3C-SVG-1.1-SE/color-prop-05-t.svg

The actual image is larger than the expected one. If I empty the Shell::SizeTo()
implementation completely, I can make it fail differently, so I'm pretty sure
I'm building correctly.

[jochen (gone - plz use gerrit), 2013-05-14 11:41:01]

hum, what revision are you building on? If you're before r198696 the test does
indeed not pass

[mstensho (USE GERRIT), 2013-05-14 12:07:53]

ooh... yes, this patch was created a couple of weeks ago, so I was a couple of
weeks behind. I can reproduce the problem after gclient sync and re-applying my
patch.

[mstensho (USE GERRIT), 2013-05-15 11:02:43]

Address review issue + rebase master (separate commits)

[jochen (gone - plz use gerrit), 2013-05-21 08:08:07]

lgtm, thanks!

https://codereview.chromium.org/14496004/diff/17002/content/shell/shell.h
File content/shell/shell.h (right):

https://codereview.chromium.org/14496004/diff/17002/content/shell/shell.h#new...
content/shell/shell.h:246: int ui_elms_height_; // height of menubar, toolbar,
etc.
nit. please spell out "elms"

[mstensho (USE GERRIT), 2013-05-21 08:15:27]

Fixed

[commit-bot: I haz the power, 2013-05-21 10:36:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mstensho@opera.com/14496004/22001

[commit-bot: I haz the power, 2013-05-21 16:41:11]

Change committed as 201315