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

Issue 339123003: Minor cleanup in EventHandler::dragHysteresisExceeded. (Closed)

Created:
6 years, 6 months ago by pwnall-personal
Modified:
6 years, 6 months ago
Reviewers:
eae
CC:
blink-reviews, blink-reviews-events_chromium.org, dglazkov+blink, eae+blinkwatch
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Minor cleanup in EventHandler::dragHysteresisExceeded. The IntPoint version of EventHandler::dragHysteresisExceeded converts its parameter to a FloatPoint and calls the FloatPoint version. In turn, that converts the parameter back to an IntPoint, and then does the real work. Last, the IntPoint version's parameter is called floatDragViewportLocation, whereas the FloatPoint version's parameter is called dragViewportLocation. This change moves the real work in the IntPoint version, and uses the FloatPoint->IntPoint conversion code inside the old FloatPoint version to delegate from the FloatPoint version to the IntPoint version. The main goal is to reduce cognitive overhead on readers. The change also removes an obsolete comment in DragActions. The comment refers to WebDragDestinationAction, which does not currently exist in blink. BUG= Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176354

Patch Set 1 #

Patch Set 2 : (ab)using this CL to remove an invalid comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -6 lines) Patch
M Source/core/page/DragActions.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/page/EventHandler.cpp View 1 chunk +4 lines, -5 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
pwnall-personal
I saw this minor cleanup opportunity while reading the drag & drop code. Can you ...
6 years, 6 months ago (2014-06-17 20:52:33 UTC) #1
eae
LGTM
6 years, 6 months ago (2014-06-17 20:55:14 UTC) #2
pwnall-personal
The CQ bit was checked by costan@gmail.com
6 years, 6 months ago (2014-06-17 21:01:00 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/costan@gmail.com/339123003/20001
6 years, 6 months ago (2014-06-17 21:01:17 UTC) #4
commit-bot: I haz the power
Change committed as 176354
6 years, 6 months ago (2014-06-17 22:21:09 UTC) #5
pwnall-personal
6 years, 6 months ago (2014-06-17 22:24:40 UTC) #6
Message was sent while issue was closed.
On 2014/06/17 20:55:14, eae wrote:
> LGTM

Thank you for the quick review!

Powered by Google App Engine
This is Rietveld 408576698