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

Issue 1212113007: Add WebPointerProperties type. (Closed)

Created:
5 years, 5 months ago by USE eero AT chromium.org
Modified:
5 years, 5 months ago
Reviewers:
bokan, mustaq, Rick Byers
CC:
blink-reviews, dglazkov+blink, mustaq, jdduke (slow), tdresser
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Add WebPointerProperties type. This adds WebPointerProperties types as common types which is inherited by WebMouseEvent and WebTouchPoint. BUG=508283 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199091

Patch Set 1 #

Total comments: 11

Patch Set 2 : Do not rename fields, drop WebMouseProperties and WebPointerEvent #

Total comments: 13

Patch Set 3 : Comments, do not touch radiusX/Y, relocate WebPointerProperties.h #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -17 lines) Patch
M Source/web/WebInputEvent.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
A public/platform/WebPointerProperties.h View 1 2 1 chunk +39 lines, -0 lines 0 comments Download
M public/web/WebInputEvent.h View 1 2 5 chunks +5 lines, -11 lines 0 comments Download
M public/web/WebTouchPoint.h View 1 2 2 chunks +7 lines, -5 lines 0 comments Download

Messages

Total messages: 38 (11 generated)
USE eero AT chromium.org
Is this the kind of solution you were thinking in https://codereview.chromium.org/1192463008/#msg7? The idea is to ...
5 years, 5 months ago (2015-07-06 16:40:50 UTC) #2
USE eero AT chromium.org
This should of course be rebased on top of https://codereview.chromium.org/1216053014/ once it is reviewed.
5 years, 5 months ago (2015-07-06 16:43:42 UTC) #3
Rick Byers
On 2015/07/06 16:40:50, e_hakkinen wrote: > Is this the kind of solution you were thinking ...
5 years, 5 months ago (2015-07-06 16:56:19 UTC) #4
Rick Byers
This is an interesting approach, thanks. Thinking about it further I think we can make ...
5 years, 5 months ago (2015-07-06 17:22:39 UTC) #5
Rick Byers
By the way, one of my motivations for simplifying this as much as possible is ...
5 years, 5 months ago (2015-07-06 17:25:13 UTC) #6
USE eero AT chromium.org
PTAL. https://codereview.chromium.org/1212113007/diff/1/public/web/WebInputEvent.h File public/web/WebInputEvent.h (right): https://codereview.chromium.org/1212113007/diff/1/public/web/WebInputEvent.h#newcode424 public/web/WebInputEvent.h:424: class WebPointerEvent : public WebMouseEvent, public WebPointerProperties { ...
5 years, 5 months ago (2015-07-07 10:00:56 UTC) #7
mustaq
https://codereview.chromium.org/1212113007/diff/20001/public/web/WebTouchPoint.h File public/web/WebTouchPoint.h (right): https://codereview.chromium.org/1212113007/diff/20001/public/web/WebTouchPoint.h#newcode42 public/web/WebTouchPoint.h:42: class WebTouchPoint : public WebPointerProperties { May be I ...
5 years, 5 months ago (2015-07-07 18:23:11 UTC) #9
Rick Byers
On 2015/07/07 18:23:11, mustaq wrote: > https://codereview.chromium.org/1212113007/diff/20001/public/web/WebTouchPoint.h > File public/web/WebTouchPoint.h (right): > > https://codereview.chromium.org/1212113007/diff/20001/public/web/WebTouchPoint.h#newcode42 > ...
5 years, 5 months ago (2015-07-09 00:39:31 UTC) #10
Rick Byers
LGTM with nits. Since this affects the IPC types, let's make sure both blink and ...
5 years, 5 months ago (2015-07-09 00:44:27 UTC) #11
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1212113007/20001
5 years, 5 months ago (2015-07-09 00:44:52 UTC) #13
Rick Byers
Also please reference a bug in your CL description - probably one for adding stylus ...
5 years, 5 months ago (2015-07-09 00:45:33 UTC) #14
Rick Byers
https://codereview.chromium.org/1212113007/diff/20001/Source/web/WebInputEvent.cpp File Source/web/WebInputEvent.cpp (right): https://codereview.chromium.org/1212113007/diff/20001/Source/web/WebInputEvent.cpp#newcode51 Source/web/WebInputEvent.cpp:51: int pointerPropertiesData[4]; nit: I think it's preferable to just ...
5 years, 5 months ago (2015-07-09 00:50:24 UTC) #16
USE eero AT chromium.org
Will do soon. I am on the road without my git repos this week, but ...
5 years, 5 months ago (2015-07-09 08:02:58 UTC) #17
Rick Byers
On 2015/07/09 08:02:58, e_hakkinen wrote: > Will do soon. I am on the road without ...
5 years, 5 months ago (2015-07-09 14:40:27 UTC) #19
mustaq
https://codereview.chromium.org/1212113007/diff/20001/public/web/WebPointerProperties.h File public/web/WebPointerProperties.h (right): https://codereview.chromium.org/1212113007/diff/20001/public/web/WebPointerProperties.h#newcode34 public/web/WebPointerProperties.h:34: float radiusX; A quick suggestion around the touch orientation ...
5 years, 5 months ago (2015-07-09 17:47:01 UTC) #20
mustaq
I have created crbug.com/508283 to track our progress towards our final goal. Please add the ...
5 years, 5 months ago (2015-07-09 17:54:34 UTC) #21
Rick Byers
On 2015/07/09 17:47:01, mustaq wrote: > https://codereview.chromium.org/1212113007/diff/20001/public/web/WebPointerProperties.h > File public/web/WebPointerProperties.h (right): > > https://codereview.chromium.org/1212113007/diff/20001/public/web/WebPointerProperties.h#newcode34 > ...
5 years, 5 months ago (2015-07-09 18:42:02 UTC) #22
bokan
Source/web lgtm Please link the patch to a bug by adding to the description: BUG=123456
5 years, 5 months ago (2015-07-13 18:41:52 UTC) #23
bokan
https://codereview.chromium.org/1212113007/diff/20001/Source/web/WebInputEvent.cpp File Source/web/WebInputEvent.cpp (right): https://codereview.chromium.org/1212113007/diff/20001/Source/web/WebInputEvent.cpp#newcode51 Source/web/WebInputEvent.cpp:51: int pointerPropertiesData[4]; On 2015/07/09 00:50:23, Rick Byers (Out until ...
5 years, 5 months ago (2015-07-13 18:42:23 UTC) #24
bokan
https://codereview.chromium.org/1212113007/diff/20001/Source/web/WebInputEvent.cpp File Source/web/WebInputEvent.cpp (right): https://codereview.chromium.org/1212113007/diff/20001/Source/web/WebInputEvent.cpp#newcode50 Source/web/WebInputEvent.cpp:50: int mouseData[10]; Actually, shouldn't this have shrunk? Since you ...
5 years, 5 months ago (2015-07-13 18:45:18 UTC) #25
USE eero AT chromium.org
I also moved WebPointerProperties.h from public/web/ to public/platform/ as that seems to be required so ...
5 years, 5 months ago (2015-07-14 23:44:13 UTC) #26
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1212113007/40001
5 years, 5 months ago (2015-07-15 22:45:16 UTC) #28
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/71297)
5 years, 5 months ago (2015-07-16 00:27:22 UTC) #30
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1212113007/40001
5 years, 5 months ago (2015-07-16 21:46:49 UTC) #32
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 5 months ago (2015-07-16 22:33:41 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1212113007/40001
5 years, 5 months ago (2015-07-17 07:12:55 UTC) #37
commit-bot: I haz the power
5 years, 5 months ago (2015-07-17 08:35:29 UTC) #38
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199091

Powered by Google App Engine
This is Rietveld 408576698