Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(495)

Issue 16488: Add a new resizer corner overlay. (Closed)

Created:
11 years, 12 months ago by MAD
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Add a new resizer corner. To display a resize bitmap and handle the mouse interactions as requested in http://code.google.com/p/chromium/issues/detail?id=458. BUG=458 There are unfortunately two cases to handle and they must be handled separately. The first one is when there are no bottom shelf like the download bar, and the case where there is one. For the case without, we must draw on top of what we receive from WebKit, so we intercept the bitmap in RenderWidgetHostViewWin::OnPaint() so that we can draw the resize corner bitmap on top of it (taking into account whether we are in a right to left language or not). For the case where we have a bottom shelf, we use a dedicated view that we properly layout on top of the bottom shelf view (which takes care of handling the RTL language case for us). Same split for the mouse interactions. Without the bottom shelf, we must deal with it in RenderWidgetHostViewWin::OnMouseEvent() by sending the root window a WM_NCLBUTTONDOWN message with either HTBOTTOMRIGHT or HTBOTTOMLEFT (based on the RTL setting) and let the OS take care of the resizing. IF we have a bottom shelf, we must deal with the mouse interaction in BrowserView::NonClientHitTest() to either return HTBOTTOMRIGHT or HTBOTTOMLEFT (again, based on the RTL setting) and, again, let the OS take care of the resizing. More details here: http://code.google.com/p/chromium/wiki/BrowserViewResizer

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Patch Set 5 : '' #

Total comments: 6

Patch Set 6 : '' #

Total comments: 7

Patch Set 7 : '' #

Patch Set 8 : '' #

Patch Set 9 : '' #

Patch Set 10 : '' #

Patch Set 11 : '' #

Total comments: 12

Patch Set 12 : '' #

Total comments: 14

Patch Set 13 : '' #

Patch Set 14 : '' #

Patch Set 15 : '' #

Patch Set 16 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+329 lines, -168 lines) Patch
M chrome/browser/browser.h View 7 8 9 10 11 12 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/browser.cc View 7 8 9 10 11 12 3 chunks +6 lines, -2 lines 0 comments Download
M chrome/browser/browser_window.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +8 lines, -2 lines 0 comments Download
M chrome/browser/browser_window_cocoa.h View 11 12 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/browser_window_cocoa.mm View 11 12 1 chunk +5 lines, -1 line 0 comments Download
M chrome/browser/browser_window_gtk.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/browser_window_gtk.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_view_host.h View 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_view_host.cc View 7 8 9 10 11 12 7 chunks +15 lines, -6 lines 0 comments Download
M chrome/browser/renderer_host/render_view_host_delegate.h View 7 8 9 10 11 12 13 4 chunks +8 lines, -7 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_host.h View 7 8 9 10 11 12 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_host.cc View 7 8 9 10 11 12 2 chunks +6 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/render_widget_host_view_win.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_host_view_win.cc View 7 8 9 10 11 12 13 5 chunks +75 lines, -14 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.cc View 2 chunks +0 lines, -8 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents.h View 7 8 9 10 11 12 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents_delegate.h View 7 8 9 10 11 12 5 chunks +11 lines, -12 lines 0 comments Download
M chrome/browser/tab_contents/web_contents.h View 7 8 9 10 11 12 3 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/tab_contents/web_contents.cc View 7 8 9 10 11 12 7 chunks +19 lines, -14 lines 0 comments Download
M chrome/browser/views/download_shelf_view.h View 4 5 6 7 8 9 10 11 12 3 chunks +8 lines, -2 lines 0 comments Download
M chrome/browser/views/download_shelf_view.cc View 4 5 6 7 8 9 10 11 12 13 3 chunks +7 lines, -1 line 0 comments Download
M chrome/browser/views/frame/browser_view.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/views/frame/browser_view.cc View 1 2 3 4 5 6 7 8 9 10 11 12 11 chunks +108 lines, -7 lines 0 comments Download
M chrome/common/render_messages_internal.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +5 lines, -3 lines 0 comments Download
M chrome/common/slide_animation.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/common/temp_scaffolding_stubs.h View 10 11 12 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/renderer/render_widget.h View 7 8 9 10 11 12 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/renderer/render_widget.cc View 7 8 9 10 11 12 2 chunks +6 lines, -3 lines 0 comments Download
M chrome/test/test_browser_window.h View 5 6 7 8 9 10 11 12 2 chunks +2 lines, -1 line 0 comments Download
D chrome/views/resize_corner.h View 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -28 lines 0 comments Download
D chrome/views/resize_corner.cc View 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -33 lines 0 comments Download
M chrome/views/views.vcproj View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -8 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
Jo
Nice job, but there are few issues: - resize corner paint over scroll just after ...
11 years, 11 months ago (2009-01-01 18:57:50 UTC) #1
MAD
On 2009/01/01 18:57:50, Jo wrote: > Nice job, but there are few issues: > - ...
11 years, 11 months ago (2009-01-01 19:37:01 UTC) #2
MAD
Now that I got a chance to take a closer look, I see the bug ...
11 years, 11 months ago (2009-01-05 15:23:58 UTC) #3
Jo
I think I didn't well explain the "smooth change" thing. The problem is that when ...
11 years, 11 months ago (2009-01-05 22:03:32 UTC) #4
MAD
Salut, this version fixes the problem that Jorat had found when he tried the initial ...
11 years, 11 months ago (2009-01-16 00:41:46 UTC) #5
Jo
That's look just fine now. gj.
11 years, 11 months ago (2009-01-16 04:18:16 UTC) #6
MAD
On 2009/01/16 04:18:16, Jo wrote: > That's look just fine now. gj. Thanks!
11 years, 11 months ago (2009-01-16 20:25:26 UTC) #7
Jo
I made a mistake earlier. Only one bug have been fixed. All other were still ...
11 years, 11 months ago (2009-01-18 01:00:00 UTC) #8
MAD
Good catch, thanks for the suggestion... I'm testing this now and will upload an update ...
11 years, 11 months ago (2009-01-19 19:17:47 UTC) #9
MAD
OK, uploaded a new patch with a change almost as the one Jo suggested... Except ...
11 years, 11 months ago (2009-01-19 23:24:04 UTC) #10
Ben Goodger (Google)
So in summary, your design should have the IPC message handler on the RVH object, ...
11 years, 11 months ago (2009-01-20 21:59:28 UTC) #11
MAD
OK, just to make sure I understand, here's what I would need to do: Move ...
11 years, 11 months ago (2009-01-21 19:54:20 UTC) #12
MAD
OK, one more try... After removing the IPC message to fetch the resizer rect, and ...
11 years, 10 months ago (2009-02-04 16:40:43 UTC) #13
brettw
Here's another pass... http://codereview.chromium.org/16488/diff/1884/941 File chrome/browser/browser_window.h (right): http://codereview.chromium.org/16488/diff/1884/941#newcode106 Line 106: // Returns the rect where ...
11 years, 10 months ago (2009-02-04 18:46:50 UTC) #14
MAD
Cool... Made all changes as requested... Have a little question at the end, whether we ...
11 years, 10 months ago (2009-02-05 02:44:22 UTC) #15
jeremy
http://codereview.chromium.org/16488/diff/1884/947 File chrome/browser/renderer_host/render_widget_host_view_win.cc (right): http://codereview.chromium.org/16488/diff/1884/947#newcode267 Line 267: SetCursor(kCursorResizeRigth); kCursorResizeRight
11 years, 10 months ago (2009-02-05 07:19:27 UTC) #16
MAD
Thanks! BYE MAD http://codereview.chromium.org/16488/diff/1884/947 File chrome/browser/renderer_host/render_widget_host_view_win.cc (right): http://codereview.chromium.org/16488/diff/1884/947#newcode267 Line 267: SetCursor(kCursorResizeRigth); On 2009/02/05 07:19:27, jeremy ...
11 years, 10 months ago (2009-02-05 14:34:23 UTC) #17
brettw
I think this basically LGTM. I'm CCing Ben to see if he has any comments ...
11 years, 10 months ago (2009-02-05 20:13:56 UTC) #18
Ben Goodger (Google)
I just have nits. This change LGTM with fixes for these and Brett's comments. http://codereview.chromium.org/16488/diff/979/988 ...
11 years, 10 months ago (2009-02-05 23:44:57 UTC) #19
MAD
11 years, 10 months ago (2009-02-06 16:42:01 UTC) #20
Fixed all the nit & uploaded last changes...

Will now sync, build, test, and finally... COMMIT!!! :-)

Thanks!

BYE
MAD

http://codereview.chromium.org/16488/diff/979/988
File chrome/browser/renderer_host/render_widget_host_view_win.cc (right):

http://codereview.chromium.org/16488/diff/979/988#newcode262
Line 262: Contains(pt.x, pt.y)) {
On 2009/02/05 23:44:57, Ben Goodger wrote:
> nit: this wrapping looks weird.

Done.

http://codereview.chromium.org/16488/diff/979/988#newcode350
Line 350: (const_cast<SkBitmap&>(canvas.getDevice()->accessBitmap(true))).
Sure... Again, I copied that code from somewhere else, should I also fix it at
the other place?

Same with the grit comment above the resource file inclusion of you other
comment.

http://codereview.chromium.org/16488/diff/979/988#newcode772
Line 772: if (message == WM_LBUTTONDOWN) {
Ho... Yes... Good point...
Done! :-)

http://codereview.chromium.org/16488/diff/979/992
File chrome/browser/tab_contents/web_contents.cc (right):

http://codereview.chromium.org/16488/diff/979/992#newcode1272
Line 1272: else
On 2009/02/05 23:44:57, Ben Goodger wrote:
> nit: no else after return

Done.

http://codereview.chromium.org/16488/diff/979/996
File chrome/browser/views/download_shelf_view.h (right):

http://codereview.chromium.org/16488/diff/979/996#newcode75
Line 75: bool IsShowing();
On 2009/02/05 23:44:57, Ben Goodger wrote:
> is SlideAnimation::IsShowing() non-const?

Yes it was, but now I changed it :-)
Done.

http://codereview.chromium.org/16488/diff/979/997
File chrome/browser/views/frame/browser_view.cc (right):

http://codereview.chromium.org/16488/diff/979/997#newcode163
Line 163: DISALLOW_EVIL_CONSTRUCTORS(ResizeCorner);
On 2009/02/05 20:13:56, brettw wrote:
> Use DISALLOW_COPY_AND_ASSIGN instead.

Done.

http://codereview.chromium.org/16488/diff/979/997#newcode984
Line 984: else
On 2009/02/05 23:44:57, Ben Goodger wrote:
> nit: no else after return.

Done.

Powered by Google App Engine
This is Rietveld 408576698