|
|
Created:
9 years, 6 months ago by jianli Modified:
9 years, 3 months ago CC:
chromium-reviews, dcheng Base URL:
svn://chrome-svn/chrome/trunk/src/ Visibility:
Public. |
DescriptionFix the issue that a panel window cannot be shrink to very small.
BUG=none
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99837
Patch Set 1 #
Total comments: 1
Patch Set 2 : '' #Patch Set 3 : '' #Messages
Total messages: 22 (0 generated)
http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc File views/widget/native_widget_win.cc (right): http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.... views/widget/native_widget_win.cc:2187: if (GetWidget()->frame_type() == Widget::FRAME_TYPE_FORCE_CUSTOM) { This is incorrect. Custom frame windows need to have standard window styles, otherwise various sizing functionality will not work.
For the panel, the moving and sizing are all controlled by panel manager. From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the following 4 scenarios: 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. 2) ConstrainedWindowFrameView. It seems none is using it. 3) PanelBrowserFrameView. This is our Panel stuff. 4) AppPanelBrowserFrameView. This is deprecated. So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I can introduce a new frame type to mean it. On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: > > http://codereview.chromium.**org/7248018/diff/1/views/** > widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> > File views/widget/native_widget_**win.cc (right): > > http://codereview.chromium.**org/7248018/diff/1/views/** > widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> > views/widget/native_widget_**win.cc:2187: if (GetWidget()->frame_type() == > > Widget::FRAME_TYPE_FORCE_**CUSTOM) { > This is incorrect. Custom frame windows need to have standard window > styles, otherwise various sizing functionality will not work. > > > http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >
What behavior are you trying to obtain? -Ben On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: > For the panel, the moving and sizing are all controlled by panel manager. > > From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the following 4 > scenarios: > > 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. > 2) ConstrainedWindowFrameView. It seems none is using it. > 3) PanelBrowserFrameView. This is our Panel stuff. > 4) AppPanelBrowserFrameView. This is deprecated. > > So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I can > introduce a new frame type to mean it. > > > On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: > >> >> http://codereview.chromium.**org/7248018/diff/1/views/** >> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >> File views/widget/native_widget_**win.cc (right): >> >> http://codereview.chromium.**org/7248018/diff/1/views/** >> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >> views/widget/native_widget_**win.cc:2187: if (GetWidget()->frame_type() >> == >> >> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >> This is incorrect. Custom frame windows need to have standard window >> styles, otherwise various sizing functionality will not work. >> >> >> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >> > >
We would like to change the height of the window to be as small as 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows will automatically enforce a larger size even when we specify a small size. On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > What behavior are you trying to obtain? > > -Ben > > > On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: > >> For the panel, the moving and sizing are all controlled by panel manager. >> >> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the following 4 >> scenarios: >> >> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. >> 2) ConstrainedWindowFrameView. It seems none is using it. >> 3) PanelBrowserFrameView. This is our Panel stuff. >> 4) AppPanelBrowserFrameView. This is deprecated. >> >> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I can >> introduce a new frame type to mean it. >> >> >> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >> >>> >>> http://codereview.chromium.**org/7248018/diff/1/views/** >>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>> File views/widget/native_widget_**win.cc (right): >>> >>> http://codereview.chromium.**org/7248018/diff/1/views/** >>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>> views/widget/native_widget_**win.cc:2187: if (GetWidget()->frame_type() >>> == >>> >>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>> This is incorrect. Custom frame windows need to have standard window >>> styles, otherwise various sizing functionality will not work. >>> >>> >>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>> >> >> >
And yet you still want to provide a non-client view, correct? -Ben On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: > We would like to change the height of the window to be as small as 3-pixel. > It seems that if we pass styles like WS_SYSMENU or others, Windows will > automatically enforce a larger size even when we specify a small size. > > > On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > >> What behavior are you trying to obtain? >> >> -Ben >> >> >> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: >> >>> For the panel, the moving and sizing are all controlled by panel manager. >>> >>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the following 4 >>> scenarios: >>> >>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. >>> 2) ConstrainedWindowFrameView. It seems none is using it. >>> 3) PanelBrowserFrameView. This is our Panel stuff. >>> 4) AppPanelBrowserFrameView. This is deprecated. >>> >>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I can >>> introduce a new frame type to mean it. >>> >>> >>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>> >>>> >>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>> File views/widget/native_widget_**win.cc (right): >>>> >>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>> views/widget/native_widget_**win.cc:2187: if (GetWidget()->frame_type() >>>> == >>>> >>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>> This is incorrect. Custom frame windows need to have standard window >>>> styles, otherwise various sizing functionality will not work. >>>> >>>> >>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>> >>> >>> >> >
Does changing the handling of WM_GETMINMAXINFO help? -Ben On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > And yet you still want to provide a non-client view, correct? > > -Ben > > > On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: > >> We would like to change the height of the window to be as small as >> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >> will automatically enforce a larger size even when we specify a small size. >> >> >> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >> >>> What behavior are you trying to obtain? >>> >>> -Ben >>> >>> >>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: >>> >>>> For the panel, the moving and sizing are all controlled by panel >>>> manager. >>>> >>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the following >>>> 4 scenarios: >>>> >>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. >>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>> >>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I can >>>> introduce a new frame type to mean it. >>>> >>>> >>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>> >>>>> >>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>> File views/widget/native_widget_**win.cc (right): >>>>> >>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>> views/widget/native_widget_**win.cc:2187: if >>>>> (GetWidget()->frame_type() == >>>>> >>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>> This is incorrect. Custom frame windows need to have standard window >>>>> styles, otherwise various sizing functionality will not work. >>>>> >>>>> >>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>> >>>> >>>> >>> >> >
Shouldn't panels be all client area, and not have a nonclient portion? I agree that WS_SYSMENU and WS_CAPTION don't make sense for a panel, but neither does having a nonclient area.
They have a caption... the title bar. -Ben On Fri, Jun 24, 2011 at 3:08 PM, <pkasting@chromium.org> wrote: > Shouldn't panels be all client area, and not have a nonclient portion? I > agree > that WS_SYSMENU and WS_CAPTION don't make sense for a panel, but neither > does > having a nonclient area. > > > http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >
On 2011/06/24 22:09:41, Ben Goodger (Google) wrote: > They have a caption... the title bar. OK... guess I have the wrong mental image of what a panel looks like. I was thinking a bubble with a close box but not a real title bar. Ignore me :)
Think gmail chat moles, but on your desktop. They might even be (vertically?) resizable. The panel manager does some amount of auto-management and supports minimizing them, so I can see how some of the windows defaults might interfere. The trick is to find the right way to suppress them while not breaking other stuff. -Ben On Fri, Jun 24, 2011 at 3:11 PM, <pkasting@chromium.org> wrote: > On 2011/06/24 22:09:41, Ben Goodger (Google) wrote: > >> They have a caption... the title bar. >> > > OK... guess I have the wrong mental image of what a panel looks like. I > was > thinking a bubble with a close box but not a real title bar. Ignore me :) > > > http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >
On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > Does changing the handling of WM_GETMINMAXINFO help? Nope. I intercepted all WIN32 messages sent to a window when it is being resized and none of them help. So I think Windows adds some size automatically if certain window style is included. > > -Ben > > > On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > >> And yet you still want to provide a non-client view, correct? > > Yes, we still want non-client view for the panel in minimized state (3-pixel line). > >> -Ben >> >> >> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: >> >>> We would like to change the height of the window to be as small as >>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>> will automatically enforce a larger size even when we specify a small size. >>> >>> >>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >>> >>>> What behavior are you trying to obtain? >>>> >>>> -Ben >>>> >>>> >>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: >>>> >>>>> For the panel, the moving and sizing are all controlled by panel >>>>> manager. >>>>> >>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the following >>>>> 4 scenarios: >>>>> >>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. >>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>> >>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I >>>>> can introduce a new frame type to mean it. >>>>> >>>>> >>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>> >>>>>> >>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>> File views/widget/native_widget_**win.cc (right): >>>>>> >>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>> (GetWidget()->frame_type() == >>>>>> >>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>> This is incorrect. Custom frame windows need to have standard window >>>>>> styles, otherwise various sizing functionality will not work. >>>>>> >>>>>> >>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>> >>>>> >>>>> >>>> >>> >> >
It may be worth making it possible for TYPE_POPUP windows to have a NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be required though, since currently all TYPE_POPUP windows don't have a NCV. -Ben On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: > > > On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > >> Does changing the handling of WM_GETMINMAXINFO help? > > > Nope. I intercepted all WIN32 messages sent to a window when it is being > resized and none of them help. So I think Windows adds some size > automatically if certain window style is included. > >> >> -Ben >> >> >> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >> >>> And yet you still want to provide a non-client view, correct? >> >> > Yes, we still want non-client view for the panel in minimized state > (3-pixel line). > >> >>> -Ben >>> >>> >>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: >>> >>>> We would like to change the height of the window to be as small as >>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>> will automatically enforce a larger size even when we specify a small size. >>>> >>>> >>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) <ben@chromium.org >>>> > wrote: >>>> >>>>> What behavior are you trying to obtain? >>>>> >>>>> -Ben >>>>> >>>>> >>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: >>>>> >>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>> manager. >>>>>> >>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>> following 4 scenarios: >>>>>> >>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. >>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>> >>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I >>>>>> can introduce a new frame type to mean it. >>>>>> >>>>>> >>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>> >>>>>>> >>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>> >>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>> (GetWidget()->frame_type() == >>>>>>> >>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>> This is incorrect. Custom frame windows need to have standard window >>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>> >>>>>>> >>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> >
Sounds like a good idea. But I do not want to interfere with all current TYPE_POPUP widgets that do not have a NCV. How about that I add a new type to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? Jian On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > It may be worth making it possible for TYPE_POPUP windows to have a > NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be > required though, since currently all TYPE_POPUP windows don't have a NCV. > > -Ben > > > On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: > >> >> >> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >> >>> Does changing the handling of WM_GETMINMAXINFO help? >> >> >> Nope. I intercepted all WIN32 messages sent to a window when it is being >> resized and none of them help. So I think Windows adds some size >> automatically if certain window style is included. >> >>> >>> -Ben >>> >>> >>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >>> >>>> And yet you still want to provide a non-client view, correct? >>> >>> >> Yes, we still want non-client view for the panel in minimized state >> (3-pixel line). >> >>> >>>> -Ben >>>> >>>> >>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: >>>> >>>>> We would like to change the height of the window to be as small as >>>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>>> will automatically enforce a larger size even when we specify a small size. >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>> ben@chromium.org> wrote: >>>>> >>>>>> What behavior are you trying to obtain? >>>>>> >>>>>> -Ben >>>>>> >>>>>> >>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org> wrote: >>>>>> >>>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>>> manager. >>>>>>> >>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>> following 4 scenarios: >>>>>>> >>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is passed. >>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>> >>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I >>>>>>> can introduce a new frame type to mean it. >>>>>>> >>>>>>> >>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>> >>>>>>>> >>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>> >>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>> (GetWidget()->frame_type() == >>>>>>>> >>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>> This is incorrect. Custom frame windows need to have standard window >>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>> >>>>>>>> >>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
We already NULL check the non-client view in places necessary to preserve that control path, so it should be safe, but let me know if you run into any issues. -Ben On Fri, Jun 24, 2011 at 3:56 PM, Jian Li <jianli@chromium.org> wrote: > Sounds like a good idea. But I do not want to interfere with all current > TYPE_POPUP widgets that do not have a NCV. How about that I add a new type > to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? > > Jian > > > On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > >> It may be worth making it possible for TYPE_POPUP windows to have a >> NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be >> required though, since currently all TYPE_POPUP windows don't have a NCV. >> >> -Ben >> >> >> On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: >> >>> >>> >>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >>> >>>> Does changing the handling of WM_GETMINMAXINFO help? >>> >>> >>> Nope. I intercepted all WIN32 messages sent to a window when it is being >>> resized and none of them help. So I think Windows adds some size >>> automatically if certain window style is included. >>> >>>> >>>> -Ben >>>> >>>> >>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org >>>> > wrote: >>>> >>>>> And yet you still want to provide a non-client view, correct? >>>> >>>> >>> Yes, we still want non-client view for the panel in minimized state >>> (3-pixel line). >>> >>>> >>>>> -Ben >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: >>>>> >>>>>> We would like to change the height of the window to be as small as >>>>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>>>> will automatically enforce a larger size even when we specify a small size. >>>>>> >>>>>> >>>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>>> ben@chromium.org> wrote: >>>>>> >>>>>>> What behavior are you trying to obtain? >>>>>>> >>>>>>> -Ben >>>>>>> >>>>>>> >>>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>> >>>>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>>>> manager. >>>>>>>> >>>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>>> following 4 scenarios: >>>>>>>> >>>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is >>>>>>>> passed. >>>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>>> >>>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or I >>>>>>>> can introduce a new frame type to mean it. >>>>>>>> >>>>>>>> >>>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>>> >>>>>>>>> >>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>>> >>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>>> (GetWidget()->frame_type() == >>>>>>>>> >>>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>>> This is incorrect. Custom frame windows need to have standard >>>>>>>>> window >>>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>>> >>>>>>>>> >>>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
Do you mean passing TYPE_POPUP from BrowserFrame::InitBrowserFrame to Widget::Init and creating NonClientView when we're creating a PanelBrowserView? Currently we create a BrowserFrame from PanelBrowserView. BrowserFrame::InitBrowserFrame passes the default TYPE_WINDOW to the Widget::Init. PanelBrowserView* view = new PanelBrowserView(browser, panel, bounds); BrowserFrame* frame = new BrowserFrame(view); frame->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM); frame->InitBrowserFrame(); In Widget::Init, it creates NonClientView if the type is TYPE_WINDOW. Do we want to introduce PanelBrowserFrame and override InitBrowserFrame in order to pass TYPE_POPUP to Widget::Init? In Widget::Init, do we want to create NonClientView when the type is TYPE_WINDOW or TYPE_POPUP? Thanks, Jian On Fri, Jun 24, 2011 at 3:58 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > We already NULL check the non-client view in places necessary to preserve > that control path, so it should be safe, but let me know if you run into any > issues. > > -Ben > > > On Fri, Jun 24, 2011 at 3:56 PM, Jian Li <jianli@chromium.org> wrote: > >> Sounds like a good idea. But I do not want to interfere with all current >> TYPE_POPUP widgets that do not have a NCV. How about that I add a new type >> to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? >> >> Jian >> >> >> On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >> >>> It may be worth making it possible for TYPE_POPUP windows to have a >>> NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be >>> required though, since currently all TYPE_POPUP windows don't have a NCV. >>> >>> -Ben >>> >>> >>> On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: >>> >>>> >>>> >>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) <ben@chromium.org >>>> > wrote: >>>> >>>>> Does changing the handling of WM_GETMINMAXINFO help? >>>> >>>> >>>> Nope. I intercepted all WIN32 messages sent to a window when it is being >>>> resized and none of them help. So I think Windows adds some size >>>> automatically if certain window style is included. >>>> >>>>> >>>>> -Ben >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>> ben@chromium.org> wrote: >>>>> >>>>>> And yet you still want to provide a non-client view, correct? >>>>> >>>>> >>>> Yes, we still want non-client view for the panel in minimized state >>>> (3-pixel line). >>>> >>>>> >>>>>> -Ben >>>>>> >>>>>> >>>>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org> wrote: >>>>>> >>>>>>> We would like to change the height of the window to be as small as >>>>>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>>>>> will automatically enforce a larger size even when we specify a small size. >>>>>>> >>>>>>> >>>>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>>>> ben@chromium.org> wrote: >>>>>>> >>>>>>>> What behavior are you trying to obtain? >>>>>>>> >>>>>>>> -Ben >>>>>>>> >>>>>>>> >>>>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>> >>>>>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>>>>> manager. >>>>>>>>> >>>>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>>>> following 4 scenarios: >>>>>>>>> >>>>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is >>>>>>>>> passed. >>>>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>>>> >>>>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or >>>>>>>>> I can introduce a new frame type to mean it. >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>>>> >>>>>>>>>> >>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>>>> >>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>>>> (GetWidget()->frame_type() == >>>>>>>>>> >>>>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>>>> This is incorrect. Custom frame windows need to have standard >>>>>>>>>> window >>>>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
I am suspicious of changing the style. I feel like there may be some unintended side effects. My belief is that there will be some way to control the minimum size of the window regardless of its styles. Have you explored this using an isolated sample win32 app? I would be especially interested to hear if you have played with things like WM_NCCALCSIZE etc. -Ben On Mon, Jun 27, 2011 at 2:48 PM, Jian Li <jianli@chromium.org> wrote: > Do you mean passing TYPE_POPUP from BrowserFrame::InitBrowserFrame to > Widget::Init and creating NonClientView when we're creating > a PanelBrowserView? > > Currently we create a BrowserFrame from PanelBrowserView. > BrowserFrame::InitBrowserFrame passes the default TYPE_WINDOW to the > Widget::Init. > PanelBrowserView* view = new PanelBrowserView(browser, panel, bounds); > BrowserFrame* frame = new BrowserFrame(view); > frame->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM); > frame->InitBrowserFrame(); > In Widget::Init, it creates NonClientView if the type is TYPE_WINDOW. > > Do we want to introduce PanelBrowserFrame and override InitBrowserFrame in > order to pass TYPE_POPUP to Widget::Init? In Widget::Init, do we want to > create NonClientView when the type is TYPE_WINDOW or TYPE_POPUP? > > Thanks, > > Jian > > On Fri, Jun 24, 2011 at 3:58 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > >> We already NULL check the non-client view in places necessary to preserve >> that control path, so it should be safe, but let me know if you run into any >> issues. >> >> -Ben >> >> >> On Fri, Jun 24, 2011 at 3:56 PM, Jian Li <jianli@chromium.org> wrote: >> >>> Sounds like a good idea. But I do not want to interfere with all current >>> TYPE_POPUP widgets that do not have a NCV. How about that I add a new type >>> to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? >>> >>> Jian >>> >>> >>> On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >>> >>>> It may be worth making it possible for TYPE_POPUP windows to have a >>>> NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be >>>> required though, since currently all TYPE_POPUP windows don't have a NCV. >>>> >>>> -Ben >>>> >>>> >>>> On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: >>>> >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>> ben@chromium.org> wrote: >>>>> >>>>>> Does changing the handling of WM_GETMINMAXINFO help? >>>>> >>>>> >>>>> Nope. I intercepted all WIN32 messages sent to a window when it is >>>>> being resized and none of them help. So I think Windows adds some size >>>>> automatically if certain window style is included. >>>>> >>>>>> >>>>>> -Ben >>>>>> >>>>>> >>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>> ben@chromium.org> wrote: >>>>>> >>>>>>> And yet you still want to provide a non-client view, correct? >>>>>> >>>>>> >>>>> Yes, we still want non-client view for the panel in minimized state >>>>> (3-pixel line). >>>>> >>>>>> >>>>>>> -Ben >>>>>>> >>>>>>> >>>>>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>> >>>>>>>> We would like to change the height of the window to be as small as >>>>>>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>>>>>> will automatically enforce a larger size even when we specify a small size. >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>>>>> ben@chromium.org> wrote: >>>>>>>> >>>>>>>>> What behavior are you trying to obtain? >>>>>>>>> >>>>>>>>> -Ben >>>>>>>>> >>>>>>>>> >>>>>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>>> >>>>>>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>>>>>> manager. >>>>>>>>>> >>>>>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>>>>> following 4 scenarios: >>>>>>>>>> >>>>>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is >>>>>>>>>> passed. >>>>>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>>>>> >>>>>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. Or >>>>>>>>>> I can introduce a new frame type to mean it. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>>>>> >>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>>>>> (GetWidget()->frame_type() == >>>>>>>>>>> >>>>>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>>>>> This is incorrect. Custom frame windows need to have standard >>>>>>>>>>> window >>>>>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
I just explored it with a sample win32 app and found out that we can minimize the window to 3-line pixels if all of the followings are satisfied: 1) WS_THICKFRAME should not be passed. 2) We need to handle WM_GETMINMAXINFO. 3) We need to handle WM_NCCALCSIZE. In our code, I override PanelBrowserView::CanResize to get rid of WS_THICKFRAME. We do handle WM_NCCALCSIZE in NativeWidgetWin::OnNCCalcSize but it did not give what we want. With further investigation, I found out that GetClientAreaInsets() returned different Insets between the first time and the subsequent times that WM_NCCALCSIZE was handled. This is because non_client_view_ is created after the native window is created and first-time WM_NCCALCSIZE is handled. void Widget::Init(const InitParams& params) { ... native_widget_->InitNativeWidget(params); if (params.type == InitParams::TYPE_WINDOW) { non_client_view_ = new NonClientView; ... } } One simple hack is to move "non_client_view_ = new NonClientView" before calling "native_widget_->InitNativeWidget". Or, we can add a special getter has_client_view() to Widget and set this flag before executing "native_widget_->InitNativeWidget". Any other better idea? Thanks, Jian On Mon, Jun 27, 2011 at 2:51 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > I am suspicious of changing the style. I feel like there may be some > unintended side effects. My belief is that there will be some way to control > the minimum size of the window regardless of its styles. > > Have you explored this using an isolated sample win32 app? I would be > especially interested to hear if you have played with things like > WM_NCCALCSIZE etc. > > -Ben > > > On Mon, Jun 27, 2011 at 2:48 PM, Jian Li <jianli@chromium.org> wrote: > >> Do you mean passing TYPE_POPUP from BrowserFrame::InitBrowserFrame to >> Widget::Init and creating NonClientView when we're creating >> a PanelBrowserView? >> >> Currently we create a BrowserFrame from PanelBrowserView. >> BrowserFrame::InitBrowserFrame passes the default TYPE_WINDOW to the >> Widget::Init. >> PanelBrowserView* view = new PanelBrowserView(browser, panel, bounds); >> BrowserFrame* frame = new BrowserFrame(view); >> frame->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM); >> frame->InitBrowserFrame(); >> In Widget::Init, it creates NonClientView if the type is TYPE_WINDOW. >> >> Do we want to introduce PanelBrowserFrame and override InitBrowserFrame in >> order to pass TYPE_POPUP to Widget::Init? In Widget::Init, do we want to >> create NonClientView when the type is TYPE_WINDOW or TYPE_POPUP? >> >> Thanks, >> >> Jian >> >> On Fri, Jun 24, 2011 at 3:58 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >> >>> We already NULL check the non-client view in places necessary to preserve >>> that control path, so it should be safe, but let me know if you run into any >>> issues. >>> >>> -Ben >>> >>> >>> On Fri, Jun 24, 2011 at 3:56 PM, Jian Li <jianli@chromium.org> wrote: >>> >>>> Sounds like a good idea. But I do not want to interfere with all current >>>> TYPE_POPUP widgets that do not have a NCV. How about that I add a new type >>>> to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? >>>> >>>> Jian >>>> >>>> >>>> On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) <ben@chromium.org >>>> > wrote: >>>> >>>>> It may be worth making it possible for TYPE_POPUP windows to have a >>>>> NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be >>>>> required though, since currently all TYPE_POPUP windows don't have a NCV. >>>>> >>>>> -Ben >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: >>>>> >>>>>> >>>>>> >>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>> ben@chromium.org> wrote: >>>>>> >>>>>>> Does changing the handling of WM_GETMINMAXINFO help? >>>>>> >>>>>> >>>>>> Nope. I intercepted all WIN32 messages sent to a window when it is >>>>>> being resized and none of them help. So I think Windows adds some size >>>>>> automatically if certain window style is included. >>>>>> >>>>>>> >>>>>>> -Ben >>>>>>> >>>>>>> >>>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>>> ben@chromium.org> wrote: >>>>>>> >>>>>>>> And yet you still want to provide a non-client view, correct? >>>>>>> >>>>>>> >>>>>> Yes, we still want non-client view for the panel in minimized state >>>>>> (3-pixel line). >>>>>> >>>>>>> >>>>>>>> -Ben >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>> >>>>>>>>> We would like to change the height of the window to be as small as >>>>>>>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>>>>>>> will automatically enforce a larger size even when we specify a small size. >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>>>>>> ben@chromium.org> wrote: >>>>>>>>> >>>>>>>>>> What behavior are you trying to obtain? >>>>>>>>>> >>>>>>>>>> -Ben >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>>>> >>>>>>>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>>>>>>> manager. >>>>>>>>>>> >>>>>>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>>>>>> following 4 scenarios: >>>>>>>>>>> >>>>>>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is >>>>>>>>>>> passed. >>>>>>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>>>>>> >>>>>>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. >>>>>>>>>>> Or I can introduce a new frame type to mean it. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>>>>>> >>>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>>>>>> (GetWidget()->frame_type() == >>>>>>>>>>>> >>>>>>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>>>>>> This is incorrect. Custom frame windows need to have standard >>>>>>>>>>>> window >>>>>>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
On Mon, Jun 27, 2011 at 6:30 PM, Jian Li <jianli@chromium.org> wrote: > With further investigation, I found out that GetClientAreaInsets() returned > different Insets between the first time and the subsequent times > that WM_NCCALCSIZE was handled. This is because non_client_view_ is created > after the native window is created and first-time WM_NCCALCSIZE is handled. > This will be problematic, as I bet the NCV initialization depends on the NativeWidget being initialized (which only happens during InitNativeWidget). However this has never been a problem for us before, since the window is sized after the non-client view is created (the call to SetInitialBounds()). Did you need to change WM_NCCALCSIZE in any way? Or override/change GetClientAreaInsets? I hope not, but if so, please let me know how/why. As for Thickframe... it sounds like something that we would want... are you sure this is what is messing with you? Take a look at the window styles in WinUser.h. I wonder if it is WS_CAPTION or WS_DLGFRAME that is the cause of the problem. It seems like there could be a weakness in our style calculation in NativeWidgetWin::SetInitParams if this is the case? -Ben > void Widget::Init(const InitParams& params) { > ... > native_widget_->InitNativeWidget(params); > if (params.type == InitParams::TYPE_WINDOW) { > non_client_view_ = new NonClientView; > ... > } > } > > One simple hack is to move "non_client_view_ = new NonClientView" before > calling "native_widget_->InitNativeWidget". Or, we can add a special getter > has_client_view() to Widget and set this flag before > executing "native_widget_->InitNativeWidget". Any other better idea? > > Thanks, > > Jian > > > On Mon, Jun 27, 2011 at 2:51 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > >> I am suspicious of changing the style. I feel like there may be some >> unintended side effects. My belief is that there will be some way to control >> the minimum size of the window regardless of its styles. >> >> Have you explored this using an isolated sample win32 app? I would be >> especially interested to hear if you have played with things like >> WM_NCCALCSIZE etc. >> >> -Ben >> >> >> On Mon, Jun 27, 2011 at 2:48 PM, Jian Li <jianli@chromium.org> wrote: >> >>> Do you mean passing TYPE_POPUP from BrowserFrame::InitBrowserFrame to >>> Widget::Init and creating NonClientView when we're creating >>> a PanelBrowserView? >>> >>> Currently we create a BrowserFrame from PanelBrowserView. >>> BrowserFrame::InitBrowserFrame passes the default TYPE_WINDOW to the >>> Widget::Init. >>> PanelBrowserView* view = new PanelBrowserView(browser, panel, bounds); >>> BrowserFrame* frame = new BrowserFrame(view); >>> frame->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM); >>> frame->InitBrowserFrame(); >>> In Widget::Init, it creates NonClientView if the type is TYPE_WINDOW. >>> >>> Do we want to introduce PanelBrowserFrame and override InitBrowserFrame >>> in order to pass TYPE_POPUP to Widget::Init? In Widget::Init, do we want to >>> create NonClientView when the type is TYPE_WINDOW or TYPE_POPUP? >>> >>> Thanks, >>> >>> Jian >>> >>> On Fri, Jun 24, 2011 at 3:58 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >>> >>>> We already NULL check the non-client view in places necessary to >>>> preserve that control path, so it should be safe, but let me know if you run >>>> into any issues. >>>> >>>> -Ben >>>> >>>> >>>> On Fri, Jun 24, 2011 at 3:56 PM, Jian Li <jianli@chromium.org> wrote: >>>> >>>>> Sounds like a good idea. But I do not want to interfere with all >>>>> current TYPE_POPUP widgets that do not have a NCV. How about that I add a >>>>> new type to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? >>>>> >>>>> Jian >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) < >>>>> ben@chromium.org> wrote: >>>>> >>>>>> It may be worth making it possible for TYPE_POPUP windows to have a >>>>>> NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be >>>>>> required though, since currently all TYPE_POPUP windows don't have a NCV. >>>>>> >>>>>> -Ben >>>>>> >>>>>> >>>>>> On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>>> ben@chromium.org> wrote: >>>>>>> >>>>>>>> Does changing the handling of WM_GETMINMAXINFO help? >>>>>>> >>>>>>> >>>>>>> Nope. I intercepted all WIN32 messages sent to a window when it is >>>>>>> being resized and none of them help. So I think Windows adds some size >>>>>>> automatically if certain window style is included. >>>>>>> >>>>>>>> >>>>>>>> -Ben >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>>>> ben@chromium.org> wrote: >>>>>>>> >>>>>>>>> And yet you still want to provide a non-client view, correct? >>>>>>>> >>>>>>>> >>>>>>> Yes, we still want non-client view for the panel in minimized state >>>>>>> (3-pixel line). >>>>>>> >>>>>>>> >>>>>>>>> -Ben >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>>> >>>>>>>>>> We would like to change the height of the window to be as small as >>>>>>>>>> 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, Windows >>>>>>>>>> will automatically enforce a larger size even when we specify a small size. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>>>>>>> ben@chromium.org> wrote: >>>>>>>>>> >>>>>>>>>>> What behavior are you trying to obtain? >>>>>>>>>>> >>>>>>>>>>> -Ben >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>>>>> >>>>>>>>>>>> For the panel, the moving and sizing are all controlled by panel >>>>>>>>>>>> manager. >>>>>>>>>>>> >>>>>>>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>>>>>>> following 4 scenarios: >>>>>>>>>>>> >>>>>>>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is >>>>>>>>>>>> passed. >>>>>>>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>>>>>>> >>>>>>>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. >>>>>>>>>>>> Or I can introduce a new frame type to mean it. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>>>>>>> >>>>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>>>>>>> (GetWidget()->frame_type() == >>>>>>>>>>>>> >>>>>>>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>>>>>>> This is incorrect. Custom frame windows need to have standard >>>>>>>>>>>>> window >>>>>>>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
On Tue, Jun 28, 2011 at 1:59 PM, Ben Goodger (Google) <ben@chromium.org>wrote: > On Mon, Jun 27, 2011 at 6:30 PM, Jian Li <jianli@chromium.org> wrote: > >> With further investigation, I found out that GetClientAreaInsets() >> returned different Insets between the first time and the subsequent times >> that WM_NCCALCSIZE was handled. This is because non_client_view_ is created >> after the native window is created and first-time WM_NCCALCSIZE is handled. >> > > This will be problematic, as I bet the NCV initialization depends on the > NativeWidget being initialized (which only happens during InitNativeWidget). > What I suggest is not to change the order of NCV initialization. Instead,we just move the creation of NCV up since NCV constructor is not doing any real initialization yet. Though this stills sounds like a hack, I could not think of any other better way to solve it. I've uploaded a new patch to clarify what I suggest. > > However this has never been a problem for us before, since the window is > sized after the non-client view is created (the call to SetInitialBounds()). > Did you need to change WM_NCCALCSIZE in any way? Or override/change > GetClientAreaInsets? I hope not, but if so, please let me know how/why. > We do not need to change anything else except the one mentioned above to move the creation of NCV up. > > As for Thickframe... it sounds like something that we would want... are you > sure this is what is messing with you? Take a look at the window styles in > WinUser.h. I wonder if it is WS_CAPTION or WS_DLGFRAME that is the cause of > the problem. It seems like there could be a weakness in our style > calculation in NativeWidgetWin::SetInitParams if this is the case? > For panel, I do not think we need thick frame since we draw by ourselves and we have 1-pixel guidance for the frame. In my sample win32 app, if I pass WS_THICKFRAME, I could not change the window height to 3-pixel. > > -Ben > > >> void Widget::Init(const InitParams& params) { >> ... >> native_widget_->InitNativeWidget(params); >> if (params.type == InitParams::TYPE_WINDOW) { >> non_client_view_ = new NonClientView; >> ... >> } >> } >> >> One simple hack is to move "non_client_view_ = new NonClientView" before >> calling "native_widget_->InitNativeWidget". Or, we can add a special getter >> has_client_view() to Widget and set this flag before >> executing "native_widget_->InitNativeWidget". Any other better idea? >> >> Thanks, >> >> Jian >> >> >> On Mon, Jun 27, 2011 at 2:51 PM, Ben Goodger (Google) <ben@chromium.org>wrote: >> >>> I am suspicious of changing the style. I feel like there may be some >>> unintended side effects. My belief is that there will be some way to control >>> the minimum size of the window regardless of its styles. >>> >>> Have you explored this using an isolated sample win32 app? I would be >>> especially interested to hear if you have played with things like >>> WM_NCCALCSIZE etc. >>> >>> -Ben >>> >>> >>> On Mon, Jun 27, 2011 at 2:48 PM, Jian Li <jianli@chromium.org> wrote: >>> >>>> Do you mean passing TYPE_POPUP from BrowserFrame::InitBrowserFrame to >>>> Widget::Init and creating NonClientView when we're creating >>>> a PanelBrowserView? >>>> >>>> Currently we create a BrowserFrame from PanelBrowserView. >>>> BrowserFrame::InitBrowserFrame passes the default TYPE_WINDOW to the >>>> Widget::Init. >>>> PanelBrowserView* view = new PanelBrowserView(browser, panel, bounds); >>>> BrowserFrame* frame = new BrowserFrame(view); >>>> frame->set_frame_type(views::Widget::FRAME_TYPE_FORCE_CUSTOM); >>>> frame->InitBrowserFrame(); >>>> In Widget::Init, it creates NonClientView if the type is TYPE_WINDOW. >>>> >>>> Do we want to introduce PanelBrowserFrame and override InitBrowserFrame >>>> in order to pass TYPE_POPUP to Widget::Init? In Widget::Init, do we want to >>>> create NonClientView when the type is TYPE_WINDOW or TYPE_POPUP? >>>> >>>> Thanks, >>>> >>>> Jian >>>> >>>> On Fri, Jun 24, 2011 at 3:58 PM, Ben Goodger (Google) <ben@chromium.org >>>> > wrote: >>>> >>>>> We already NULL check the non-client view in places necessary to >>>>> preserve that control path, so it should be safe, but let me know if you run >>>>> into any issues. >>>>> >>>>> -Ben >>>>> >>>>> >>>>> On Fri, Jun 24, 2011 at 3:56 PM, Jian Li <jianli@chromium.org> wrote: >>>>> >>>>>> Sounds like a good idea. But I do not want to interfere with all >>>>>> current TYPE_POPUP widgets that do not have a NCV. How about that I add a >>>>>> new type to Widget::InitParams, like TYPE_POPUP_WITH_FRAME or TYPE_PANEL? >>>>>> >>>>>> Jian >>>>>> >>>>>> >>>>>> On Fri, Jun 24, 2011 at 3:26 PM, Ben Goodger (Google) < >>>>>> ben@chromium.org> wrote: >>>>>> >>>>>>> It may be worth making it possible for TYPE_POPUP windows to have a >>>>>>> NonClientView. Currently only TYPE_WINDOW windows do. It wouldn't be >>>>>>> required though, since currently all TYPE_POPUP windows don't have a NCV. >>>>>>> >>>>>>> -Ben >>>>>>> >>>>>>> >>>>>>> On Fri, Jun 24, 2011 at 3:24 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>>>> ben@chromium.org> wrote: >>>>>>>> >>>>>>>>> Does changing the handling of WM_GETMINMAXINFO help? >>>>>>>> >>>>>>>> >>>>>>>> Nope. I intercepted all WIN32 messages sent to a window when it is >>>>>>>> being resized and none of them help. So I think Windows adds some size >>>>>>>> automatically if certain window style is included. >>>>>>>> >>>>>>>>> >>>>>>>>> -Ben >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, Jun 24, 2011 at 3:06 PM, Ben Goodger (Google) < >>>>>>>>> ben@chromium.org> wrote: >>>>>>>>> >>>>>>>>>> And yet you still want to provide a non-client view, correct? >>>>>>>>> >>>>>>>>> >>>>>>>> Yes, we still want non-client view for the panel in minimized state >>>>>>>> (3-pixel line). >>>>>>>> >>>>>>>>> >>>>>>>>>> -Ben >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Jun 24, 2011 at 3:02 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>>>> >>>>>>>>>>> We would like to change the height of the window to be as small >>>>>>>>>>> as 3-pixel. It seems that if we pass styles like WS_SYSMENU or others, >>>>>>>>>>> Windows will automatically enforce a larger size even when we specify a >>>>>>>>>>> small size. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Fri, Jun 24, 2011 at 2:03 PM, Ben Goodger (Google) < >>>>>>>>>>> ben@chromium.org> wrote: >>>>>>>>>>> >>>>>>>>>>>> What behavior are you trying to obtain? >>>>>>>>>>>> >>>>>>>>>>>> -Ben >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Thu, Jun 23, 2011 at 2:05 PM, Jian Li <jianli@chromium.org>wrote: >>>>>>>>>>>> >>>>>>>>>>>>> For the panel, the moving and sizing are all controlled by >>>>>>>>>>>>> panel manager. >>>>>>>>>>>>> >>>>>>>>>>>>> From a search, we pass FRAME_TYPE_FORCE_**CUSTOM only in the >>>>>>>>>>>>> following 4 scenarios: >>>>>>>>>>>>> >>>>>>>>>>>>> 1) DebugToggleFrameType when "--debug-enable-frame-toggle" is >>>>>>>>>>>>> passed. >>>>>>>>>>>>> 2) ConstrainedWindowFrameView. It seems none is using it. >>>>>>>>>>>>> 3) PanelBrowserFrameView. This is our Panel stuff. >>>>>>>>>>>>> 4) AppPanelBrowserFrameView. This is deprecated. >>>>>>>>>>>>> >>>>>>>>>>>>> So it seems to be OK to use FRAME_TYPE_FORCE_CUSTOM to mean it. >>>>>>>>>>>>> Or I can introduce a new frame type to mean it. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> On Thu, Jun 23, 2011 at 1:48 PM, <ben@chromium.org> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>>>>> widget/native_widget_win.cc<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc> >>>>>>>>>>>>>> File views/widget/native_widget_**win.cc (right): >>>>>>>>>>>>>> >>>>>>>>>>>>>> http://codereview.chromium.**org/7248018/diff/1/views/** >>>>>>>>>>>>>> widget/native_widget_win.cc#**newcode2187<http://codereview.chromium.org/7248018/diff/1/views/widget/native_widget_win.cc#newcode2187> >>>>>>>>>>>>>> views/widget/native_widget_**win.cc:2187: if >>>>>>>>>>>>>> (GetWidget()->frame_type() == >>>>>>>>>>>>>> >>>>>>>>>>>>>> Widget::FRAME_TYPE_FORCE_**CUSTOM) { >>>>>>>>>>>>>> This is incorrect. Custom frame windows need to have standard >>>>>>>>>>>>>> window >>>>>>>>>>>>>> styles, otherwise various sizing functionality will not work. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> http://codereview.chromium.**org/7248018/<http://codereview.chromium.org/7248... >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
This approach looks great to me. LGTM.
If this works, LGTM. |