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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 82693006: Adding handlers for user gesture callbacks. (Closed)

Created:
7 years, 1 month ago by jww
Modified:
6 years, 10 months ago
CC:
blink-reviews, jamesr, dglazkov+blink, vabr (Chromium)
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Adding handlers for user gesture callbacks. Whenever a UserGestureToken is created, calls all registered callbacks to alert them of gesture. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162707

Patch Set 1 #

Patch Set 2 : Removed removeHandler #

Total comments: 2

Patch Set 3 : Simplified handler implementation and added unit test #

Total comments: 8

Patch Set 4 : Nits from abarth #

Patch Set 5 : Fixed compiler warnings #

Patch Set 6 : Fixed unit test bug #

Unified diffs Side-by-side diffs Delta from patch set Stats (+90 lines, -2 lines) Patch
M Source/platform/UserGestureIndicator.h View 1 2 3 chunks +8 lines, -0 lines 0 comments Download
M Source/platform/UserGestureIndicator.cpp View 1 2 3 5 chunks +19 lines, -2 lines 0 comments Download
M Source/web/WebUserGestureIndicator.cpp View 1 2 3 2 chunks +34 lines, -0 lines 0 comments Download
M Source/web/tests/WebUserGestureTokenTest.cpp View 1 2 3 4 5 2 chunks +21 lines, -0 lines 0 comments Download
M public/web/WebUserGestureIndicator.h View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
jww
abarth@, here's a first whack at user gesture callbacks. Still needs unit tests, but let ...
7 years, 1 month ago (2013-11-22 23:53:18 UTC) #1
jww
abarth@, here's a first whack at user gesture callbacks. Still needs unit tests, but let ...
7 years, 1 month ago (2013-11-22 23:53:23 UTC) #2
jww
7 years, 1 month ago (2013-11-23 02:00:08 UTC) #3
abarth-chromium
Do you really need support for multiple? This code can be much simpler if you ...
7 years, 1 month ago (2013-11-23 02:22:30 UTC) #4
jww
abarth, as per your suggestion, I've simplified the callbacks since we really only need 1. ...
7 years ago (2013-11-25 21:38:36 UTC) #5
abarth-chromium
LGTM. Some nits below. https://codereview.chromium.org/82693006/diff/90001/Source/platform/UserGestureIndicator.cpp File Source/platform/UserGestureIndicator.cpp (right): https://codereview.chromium.org/82693006/diff/90001/Source/platform/UserGestureIndicator.cpp#newcode136 Source/platform/UserGestureIndicator.cpp:136: bool onGesture = false; Can ...
7 years ago (2013-11-26 19:36:36 UTC) #6
jww
https://codereview.chromium.org/82693006/diff/90001/Source/platform/UserGestureIndicator.cpp File Source/platform/UserGestureIndicator.cpp (right): https://codereview.chromium.org/82693006/diff/90001/Source/platform/UserGestureIndicator.cpp#newcode136 Source/platform/UserGestureIndicator.cpp:136: bool onGesture = false; On 2013/11/26 19:36:37, abarth wrote: ...
7 years ago (2013-11-26 20:11:55 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jww@chromium.org/82693006/110001
7 years ago (2013-11-26 20:12:23 UTC) #8
commit-bot: I haz the power
Retried try job too often on linux_blink for step(s) blink_platform_unittests, webkit_lint, webkit_python_tests, webkit_tests, webkit_unit_tests, wtf_unittests ...
7 years ago (2013-11-26 20:52:20 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jww@chromium.org/82693006/130001
7 years ago (2013-11-26 21:18:37 UTC) #10
commit-bot: I haz the power
Retried try job too often on linux_blink for step(s) webkit_unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blink&number=4026
7 years ago (2013-11-26 22:01:01 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jww@chromium.org/82693006/150001
7 years ago (2013-11-26 22:11:07 UTC) #12
commit-bot: I haz the power
Change committed as 162707
7 years ago (2013-11-26 23:38:42 UTC) #13
jochen (gone - plz use gerrit)
This appears to be completely broken. In the autofill code this is used as if ...
6 years, 10 months ago (2014-02-13 09:41:40 UTC) #14
jochen (gone - plz use gerrit)
last but not least, we sometimes create user gestures in privileged code, e.g. extension code ...
6 years, 10 months ago (2014-02-13 10:06:58 UTC) #15
abarth-chromium
On 2014/02/13 10:06:58, jochen wrote: > last but not least, we sometimes create user gestures ...
6 years, 10 months ago (2014-02-14 01:28:54 UTC) #16
jww
6 years, 10 months ago (2014-02-14 01:34:01 UTC) #17
Message was sent while issue was closed. 
On 2014/02/14 01:28:54, abarth wrote:
> On 2014/02/13 10:06:58, jochen wrote:
> > last but not least, we sometimes create user gestures in privileged code,
e.g.
> > extension code - this will make the password manager do it's thing
completely
> > out of the blue.
> 
> Would you be willing to file a bug so we can figure out how to fix this issue?

There's a bug that is working on this issue: https://crbug.com/337429

It looks like the best solution is to actually use a different API to find out
what about user gestures. This is being handled in the CL:
https://codereview.chromium.org/163843002/

Powered by Google App Engine
This is Rietveld 408576698