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

Issue 27064: A DCHECK was validating a little to widely, moved it into the if that needed ... (Closed)

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

Description

We don't support beeing scrolled in two dimensions at a time. This can happen when zooming on an image as identified in related bug. So we split it in two phases. BUG=5873 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10568

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Total comments: 1

Patch Set 3 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -3 lines) Patch
M chrome/renderer/render_widget.cc View 1 1 chunk +5 lines, -2 lines 1 comment Download
M webkit/tools/test_shell/webwidget_host_win.cc View 1 chunk +5 lines, -1 line 1 comment Download

Messages

Total messages: 8 (0 generated)
MAD
Not sure who to ask to review this and I saw your name in the ...
11 years, 10 months ago (2009-02-23 22:09:01 UTC) #1
MAD
We don't know who introduced this DCHECK initially... It has been there since the beginning ...
11 years, 10 months ago (2009-02-24 16:34:25 UTC) #2
MAD
John pointed me to Brett who pointed me to you as the author of this ...
11 years, 10 months ago (2009-02-24 18:24:06 UTC) #3
darin (slow to review)
http://codereview.chromium.org/27064/diff/1/2 File chrome/renderer/render_widget.cc (right): http://codereview.chromium.org/27064/diff/1/2#newcode559 Line 559: DCHECK((dx && !dy) || (!dx && dy)); this ...
11 years, 10 months ago (2009-02-24 18:56:47 UTC) #4
MAD
OK, so how about this approach instead? Thanks again! BYE MAD
11 years, 10 months ago (2009-02-24 19:43:40 UTC) #5
darin (slow to review)
http://codereview.chromium.org/27064/diff/1002/8 File webkit/glue/chrome_client_impl.cc (right): http://codereview.chromium.org/27064/diff/1002/8#newcode413 Line 413: // RenderWidget::DidScrollRect() doesn't like to be scrolled both ...
11 years, 10 months ago (2009-02-24 20:33:17 UTC) #6
MAD
Like this?
11 years, 10 months ago (2009-02-25 14:50:20 UTC) #7
darin (slow to review)
11 years, 10 months ago (2009-02-26 22:15:44 UTC) #8
LGTM

http://codereview.chromium.org/27064/diff/12/14
File chrome/renderer/render_widget.cc (right):

http://codereview.chromium.org/27064/diff/12/14#newcode553
Line 553: // We only support scrolling along one axis at a time.
this works by causing us to fall through to the slow path for scrolling, which
is fine.

http://codereview.chromium.org/27064/diff/12/13
File webkit/tools/test_shell/webwidget_host_win.cc (right):

http://codereview.chromium.org/27064/diff/12/13#newcode146
Line 146: // We only support uni-directional scroll
ditto

Powered by Google App Engine
This is Rietveld 408576698