Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2016073004:
Show a visual indicator for the progress of auto-click. (Closed)
|
Created:
4 years, 6 months ago by sammiequon Modified:
4 years, 5 months ago CC:
chromium-reviews, dbeam+watch-options_chromium.org, kalyank, michaelpg+watch-options_chromium.org, sadrul, arv+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@lkgr Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionShow a visual indicator for the progress of auto-click.
BUG=610476
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/820646ff0c3aa9fb6ea4d8ff727e9f5b02b1ca2a
Cr-Commit-Position: refs/heads/master@{#401913}
Patch Set 1 #
Total comments: 10
Patch Set 2 : Moved ring rendering code to chrome/browser/chromeos/ui #
Total comments: 23
Patch Set 3 : Fixed errors from patch set 2. #
Total comments: 20
Patch Set 4 : Used a timedelta in autoclick controller instead of int. #
Total comments: 18
Patch Set 5 : Fixed Patch Set 4 errors. #
Total comments: 22
Patch Set 6 : Fixed errors from patch set 5. #
Total comments: 16
Patch Set 7 : Rebase. #Patch Set 8 : Patch set 6 fixes. #Patch Set 9 : Rebase. #Patch Set 10 : Updated material design equivalent. #Patch Set 11 : Rebase. #Messages
Total messages: 49 (21 generated)
Description was changed from ========== Fixed cpp style warnings from git cl upload. Fixed the auto-click to make it circle around the actual mouse location. Refactored a snippet of code into its own function. Changed the speed of the auto-click very fast and fast. Removed some unneeded headers. Made a change to add the shrinking animation. Removed a header which was conflicting after merging master branch. Changed the copyright to year 2016. Did some refractoring. Fixed a bug with the inner circle. First revision of ring around autoclick. Modified the amount of seconds for atuo click when pointer stops. BUG= ========== to ========== Fixed cpp style warnings from git cl upload. Fixed the auto-click to make it circle around the actual mouse location. Refactored a snippet of code into its own function. Changed the speed of the auto-click very fast and fast. Removed some unneeded headers. Made a change to add the shrinking animation. Removed a header which was conflicting after merging master branch. Changed the copyright to year 2016. Did some refractoring. Fixed a bug with the inner circle. First revision of ring around autoclick. Modified the amount of seconds for atuo click when pointer stops. BUG= CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Fixed cpp style warnings from git cl upload. Fixed the auto-click to make it circle around the actual mouse location. Refactored a snippet of code into its own function. Changed the speed of the auto-click very fast and fast. Removed some unneeded headers. Made a change to add the shrinking animation. Removed a header which was conflicting after merging master branch. Changed the copyright to year 2016. Did some refractoring. Fixed a bug with the inner circle. First revision of ring around autoclick. Modified the amount of seconds for atuo click when pointer stops. BUG= CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Changed the speed of the accessibility auto-click and added a ring(s) to determine where the clicks are going. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
sammiequon@chromium.org changed reviewers: + dmazzoni@chromium.org, jdufault@chromium.org
Ideally I'd like to share some of this code with the files in this directory: chrome/browser/chromeos/ui/accessibility_focus_ring.cc I don't really care whether this lives in ash/ or chrome/browser/ but I'd just like visual accessibility overlays to all be in one place - any thoughts from other owners?
https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_con... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_con... ash/autoclick/autoclick_controller.cc:93: autoclick_ring_display_.reset(new LongPressAutoclickRingHandler); Use trailing parens with constructors: new LongPressAutoclickRingHandler() There's actually a semantic difference between them, unfortunately. https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_con... ash/autoclick/autoclick_controller.cc:182: StopRingDisplay(NULL); nit: nullptr https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_rin... File ash/autoclick/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_rin... ash/autoclick/autoclick_ring_handler.cc:203: : gfx::LinearAnimation(kAutoclickRingFrameRateHz, NULL), Replace all NULL occurrences in this file with nullptr. https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_rin... ash/autoclick/autoclick_ring_handler.cc:211: void LongPressAutoclickRingHandler::ProcessEvent(ui::LocatedEvent* event, Does |event| ever get used? Can it be removed? It'd be nice to separate this out into two methods; one for starting and one for stopping the animation. So hopefully we can go from ProcessEvent(nullptr, delay_, true/false) to StartGesture(delay_) or StopGesture(). https://codereview.chromium.org/2016073004/diff/1/chrome/browser/resources/op... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/1/chrome/browser/resources/op... chrome/browser/resources/options/browser_options.html:953: <option value="600" Are you also going to add the actual ms value in the UI string? So something like "Extremely short (600ms)"?
sammiequon@chromium.org changed reviewers: + oshima@chromium.org
Patchset #2 (id:20001) has been deleted
@jdufault - fixed suggestions from patch 1. @dmazzoni - Rendering code is now located in chrome/browser/chromeos/ui @oshima - Please take a look. https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_con... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_con... ash/autoclick/autoclick_controller.cc:93: autoclick_ring_display_.reset(new LongPressAutoclickRingHandler); On 2016/05/31 19:06:15, jdufault wrote: > Use trailing parens with constructors: new LongPressAutoclickRingHandler() > > There's actually a semantic difference between them, unfortunately. Done. https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_con... ash/autoclick/autoclick_controller.cc:182: StopRingDisplay(NULL); On 2016/05/31 19:06:15, jdufault wrote: > nit: nullptr Done. https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_rin... File ash/autoclick/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_rin... ash/autoclick/autoclick_ring_handler.cc:203: : gfx::LinearAnimation(kAutoclickRingFrameRateHz, NULL), On 2016/05/31 19:06:15, jdufault wrote: > Replace all NULL occurrences in this file with nullptr. Done. https://codereview.chromium.org/2016073004/diff/1/ash/autoclick/autoclick_rin... ash/autoclick/autoclick_ring_handler.cc:211: void LongPressAutoclickRingHandler::ProcessEvent(ui::LocatedEvent* event, On 2016/05/31 19:06:15, jdufault wrote: > Does |event| ever get used? Can it be removed? > > It'd be nice to separate this out into two methods; one for starting and one for > stopping the animation. > > So hopefully we can go from ProcessEvent(nullptr, delay_, true/false) to > StartGesture(delay_) or StopGesture(). Done. Replaced with the suggested functions. https://codereview.chromium.org/2016073004/diff/1/chrome/browser/resources/op... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/1/chrome/browser/resources/op... chrome/browser/resources/options/browser_options.html:953: <option value="600" On 2016/05/31 19:06:15, jdufault wrote: > Are you also going to add the actual ms value in the UI string? So something > like "Extremely short (600ms)"? Done.
lgtm
https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:53: void SetAutoclickgestureDelegate(std::unique_ptr<AutoclickgestureDelegate> This doesn't look like normal formatting. Did you run git cl format? The type and param name should be on a separate line with a four-space indent. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:134: if (nullptr != autoclick_gesture_delegate_) { Yoda-style conditions are unusual in the Chrome codebase. For nullptr checks, prefer implicit boolean casts, so this will become just if (autoclick_gesture_delegate_) Also remove the {} brackets. Also do this for lines 140 and 146. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... File ash/autoclick/autoclickgesture_delegate.h (right): https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:1: // Copyright (c) 2016 The Chromium Authors. All rights reserved. What about defining this class inside of AutoclickController, such that it is externally accessible as AutoclickController::Delegate https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:13: class AutoclickgestureDelegate { gesture should be capitalized to Gesture, so AutoclickGestureDelegate. Then rename the file to autoclick_gesture_delegate.h https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:17: // The actual task of starting the display of a gesture. How about comments like: Called when an autoclick gesture begins. Called when the autoclick gesture has ended, either because the mouse moved or because the autoclick completed. The delegate doesn't have to be related to the display of the gesture; for example, it could be used in a test. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:23: virtual void SetGestureCenter(gfx::Point center) = 0; Add DISALLOW_COPY_AND_ASSIGN(AutoclickgestureDelegate) to the end of this class. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:23: virtual void SetGestureCenter(gfx::Point center) = 0; Please add a comment to SetGestureCenter. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:232: //////////////////////////////////////////////////////////////////////////////// Newline between the end of the function and the start of the comment. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:21: // on a TAP_DOWN gesture. The animation sequence consists of two parts: Remove the "TAP_DOWN" bit since this class doesn't know about events/gestures anymore. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:28: public aura::WindowObserver, Remove the LongPress bit, since the class doesn't know about that anymore. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:35: void StartGesture(int delay_ms, gfx::Point center) override; Should delay_ms be renamed to duration_ms? Let's also use a base::TimeDuration type instead of an int, then there is no worry about the type of time value stored in the int. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/resource... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/resource... chrome/browser/resources/options/browser_options.html:953: <option value="600" Add a comment here saying that if the option timeout values are updated the i18n strings also need to be updated.
Please also update the title of the CL to match the first line of the description.
jdufault - updated files to your suggestions https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:53: void SetAutoclickgestureDelegate(std::unique_ptr<AutoclickgestureDelegate> On 2016/06/03 22:02:44, jdufault wrote: > This doesn't look like normal formatting. Did you run git cl format? The type > and param name should be on a separate line with a four-space indent. Done. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:134: if (nullptr != autoclick_gesture_delegate_) { On 2016/06/03 22:02:44, jdufault wrote: > Yoda-style conditions are unusual in the Chrome codebase. > > For nullptr checks, prefer implicit boolean casts, so this will become just > > if (autoclick_gesture_delegate_) > > Also remove the {} brackets. > > Also do this for lines 140 and 146. Done. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... File ash/autoclick/autoclickgesture_delegate.h (right): https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:1: // Copyright (c) 2016 The Chromium Authors. All rights reserved. On 2016/06/03 22:02:44, jdufault wrote: > What about defining this class inside of AutoclickController, such that it is > externally accessible as > > AutoclickController::Delegate Done. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:17: // The actual task of starting the display of a gesture. On 2016/06/03 22:02:44, jdufault wrote: > How about comments like: > > Called when an autoclick gesture begins. > > Called when the autoclick gesture has ended, either because the mouse moved or > because the autoclick completed. > > The delegate doesn't have to be related to the display of the gesture; for > example, it could be used in a test. Done. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:17: // The actual task of starting the display of a gesture. On 2016/06/03 22:02:44, jdufault wrote: > How about comments like: > > Called when an autoclick gesture begins. > > Called when the autoclick gesture has ended, either because the mouse moved or > because the autoclick completed. > > The delegate doesn't have to be related to the display of the gesture; for > example, it could be used in a test. Done. https://codereview.chromium.org/2016073004/diff/40001/ash/autoclick/autoclick... ash/autoclick/autoclickgesture_delegate.h:23: virtual void SetGestureCenter(gfx::Point center) = 0; On 2016/06/03 22:02:44, jdufault wrote: > Add DISALLOW_COPY_AND_ASSIGN(AutoclickgestureDelegate) to the end of this class. Done. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:21: // on a TAP_DOWN gesture. The animation sequence consists of two parts: On 2016/06/03 22:02:45, jdufault wrote: > Remove the "TAP_DOWN" bit since this class doesn't know about events/gestures > anymore. Done. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:28: public aura::WindowObserver, On 2016/06/03 22:02:44, jdufault wrote: > Remove the LongPress bit, since the class doesn't know about that anymore. Done. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:35: void StartGesture(int delay_ms, gfx::Point center) override; On 2016/06/03 22:02:44, jdufault wrote: > Should delay_ms be renamed to duration_ms? > > Let's also use a base::TimeDuration type instead of an int, then there is no > worry about the type of time value stored in the int. I couldn't find a base::TimeDuration type, but there is a base::TimeDelta uses which gfx::Animation converts an int to using SetDuration(int) which is in the call stack for StartGesture. delay_ms changed to duration_ms. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/resource... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/resource... chrome/browser/resources/options/browser_options.html:953: <option value="600" On 2016/06/03 22:02:45, jdufault wrote: > Add a comment here saying that if the option timeout values are updated the i18n > strings also need to be updated. > Done.
It looks like the title got truncated. The title and the first line of the description should match, so you might have to reword the title/desc so it's shorter. https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/40001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:35: void StartGesture(int delay_ms, gfx::Point center) override; On 2016/06/07 18:06:16, sammiequon wrote: > On 2016/06/03 22:02:44, jdufault wrote: > > Should delay_ms be renamed to duration_ms? > > > > Let's also use a base::TimeDuration type instead of an int, then there is no > > worry about the type of time value stored in the int. > > I couldn't find a base::TimeDuration type, but there is a base::TimeDelta uses > which gfx::Animation converts an int to using SetDuration(int) which is in the > call stack for StartGesture. delay_ms changed to duration_ms. Sorry, I'm a bit confused now. How does gfx::Animation come into this? I don't think it'd be too much work to convert the entire class to use base::TimeDelta. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:52: // AutoclickController overrides. Move this into the other AutoclickController override section. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:54: std::unique_ptr<Delegate> autoclickgesture_delegate) override; Just delegate https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:82: std::unique_ptr<Delegate> autoclick_gesture_delegate_; Just delegate_ https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:90: AutoclickControllerImpl::AutoclickControllerImpl() Restore newline https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.h:44: std::unique_ptr<Delegate> autoclickgesture_delegate) = 0; Just |delegate|. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.h:52: // Set the time to wait in milliseconds from when the mouse stops movin moving? https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:29: // Overriden from ash::AutoclickController::Delegate. Newline between dtor and comment. https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:30: void StartGesture(int duration_ms, gfx::Point center) override; Make overrides private https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/resource... chrome/browser/resources/options/browser_options.html:953: <!-- i18n values need to be updated as option values are.--> This is a little confusing because it's not immediately clear that the option value is mapped to the autoclick delay. How about something along the lines of The i18n strings contain the autoclick duration; if the autoclick timing gets changed, then the 18n strings also need to be updated. https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/ui/ash/a... File chrome/browser/ui/ash/ash_init.cc (right): https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/ui/ash/a... chrome/browser/ui/ash/ash_init.cc:76: std::unique_ptr<ash::AutoclickController::Delegate>( Can you use base::WrapUnique?
https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:52: // AutoclickController overrides. On 2016/06/08 17:50:19, jdufault wrote: > Move this into the other AutoclickController override section. Done. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:54: std::unique_ptr<Delegate> autoclickgesture_delegate) override; On 2016/06/08 17:50:19, jdufault wrote: > Just delegate Done. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:82: std::unique_ptr<Delegate> autoclick_gesture_delegate_; On 2016/06/08 17:50:19, jdufault wrote: > Just delegate_ Done. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:90: AutoclickControllerImpl::AutoclickControllerImpl() On 2016/06/08 17:50:19, jdufault wrote: > Restore newline Done. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.h:44: std::unique_ptr<Delegate> autoclickgesture_delegate) = 0; On 2016/06/08 17:50:19, jdufault wrote: > Just |delegate|. Done. https://codereview.chromium.org/2016073004/diff/60001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.h:52: // Set the time to wait in milliseconds from when the mouse stops movin On 2016/06/08 17:50:19, jdufault wrote: > moving? Done. https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:29: // Overriden from ash::AutoclickController::Delegate. On 2016/06/08 17:50:19, jdufault wrote: > Newline between dtor and comment. Done. https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.h:30: void StartGesture(int duration_ms, gfx::Point center) override; On 2016/06/08 17:50:19, jdufault wrote: > Make overrides private Done. https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/resource... chrome/browser/resources/options/browser_options.html:953: <!-- i18n values need to be updated as option values are.--> On 2016/06/08 17:50:20, jdufault wrote: > This is a little confusing because it's not immediately clear that the option > value is mapped to the autoclick delay. How about something along the lines of > > The i18n strings contain the autoclick duration; if the autoclick > timing gets changed, then the 18n strings also need to be updated. Done. https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/ui/ash/a... File chrome/browser/ui/ash/ash_init.cc (right): https://codereview.chromium.org/2016073004/diff/60001/chrome/browser/ui/ash/a... chrome/browser/ui/ash/ash_init.cc:76: std::unique_ptr<ash::AutoclickController::Delegate>( On 2016/06/08 17:50:20, jdufault wrote: > Can you use base::WrapUnique? Done.
https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:90: static_cast<int64_t>(kDefaultAutoclickDelayMs))), Make kDefaultAutoclickDelayMs a base::TimeDelta value and then just invoke the copy constructor here, so it will be delay_(kDefaultAutoclickDelay) https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.h:30: // Called when the gesture has ended, Cleanup the text wrapping so the first line is as long as possible. https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... File ash/autoclick/autoclick_unittest.cc (right): https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... ash/autoclick/autoclick_unittest.cc:69: GetAutoclickController()->SetAutoclickDelay(base::TiemDelta()); It looks like this is misspelled. You can compile and run this code using the ash_unittests target. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... File chrome/browser/chromeos/accessibility/accessibility_manager.cc (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/accessibility/accessibility_manager.cc:864: static_cast<int64_t>(autoclick_delay_ms_))); Parse the time value into a TimeDelta instance when you get it from the prefs system. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:107: // View of the AutoclickRingHandler. Draws the actual contents and Fix wrapping https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:231: //////////////////////////////////////////////////////////////////////////////// Please add a newline before this comment block starts. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:246: int delay_ms = static_cast<int>(delay.InMilliseconds()); Use brace initialization for casting numeric types. So probably something like int{delay.InMilliseconds()} https://google.github.io/styleguide/cppguide.html#Casting https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/resource... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/resource... chrome/browser/resources/options/browser_options.html:958: <!-- 18n strings contain the autoclick duration; if the autoclick Missing the i in i18n https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/ui/ash/a... File chrome/browser/ui/ash/ash_init.cc (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/ui/ash/a... chrome/browser/ui/ash/ash_init.cc:76: base::WrapUnique(new chromeos::AutoclickRingHandler)); Add trailing () to new AutoclickRingHandler.
Description was changed from ========== Changed the speed of the accessibility auto-click and added a ring(s) to determine where the clicks are going. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Upgrades to auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.cc (right): https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.cc:90: static_cast<int64_t>(kDefaultAutoclickDelayMs))), On 2016/06/09 16:41:26, jdufault wrote: > Make kDefaultAutoclickDelayMs a base::TimeDelta value and then just invoke the > copy constructor here, so it will be > > delay_(kDefaultAutoclickDelay) Done. https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... ash/autoclick/autoclick_controller.h:30: // Called when the gesture has ended, On 2016/06/09 16:41:26, jdufault wrote: > Cleanup the text wrapping so the first line is as long as possible. Done. https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... File ash/autoclick/autoclick_unittest.cc (right): https://codereview.chromium.org/2016073004/diff/80001/ash/autoclick/autoclick... ash/autoclick/autoclick_unittest.cc:69: GetAutoclickController()->SetAutoclickDelay(base::TiemDelta()); On 2016/06/09 16:41:26, jdufault wrote: > It looks like this is misspelled. You can compile and run this code using the > ash_unittests target. Done. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... File chrome/browser/chromeos/accessibility/accessibility_manager.cc (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/accessibility/accessibility_manager.cc:864: static_cast<int64_t>(autoclick_delay_ms_))); On 2016/06/09 16:41:26, jdufault wrote: > Parse the time value into a TimeDelta instance when you get it from the prefs > system. Done. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:107: // View of the AutoclickRingHandler. Draws the actual contents and On 2016/06/09 16:41:26, jdufault wrote: > Fix wrapping Done. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:231: //////////////////////////////////////////////////////////////////////////////// On 2016/06/09 16:41:26, jdufault wrote: > Please add a newline before this comment block starts. Done. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/chromeos... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:246: int delay_ms = static_cast<int>(delay.InMilliseconds()); On 2016/06/09 16:41:26, jdufault wrote: > Use brace initialization for casting numeric types. > > So probably something like int{delay.InMilliseconds()} > > https://google.github.io/styleguide/cppguide.html#Casting Done. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/resource... File chrome/browser/resources/options/browser_options.html (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/resource... chrome/browser/resources/options/browser_options.html:958: <!-- 18n strings contain the autoclick duration; if the autoclick On 2016/06/09 16:41:26, jdufault wrote: > Missing the i in i18n Done. https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/ui/ash/a... File chrome/browser/ui/ash/ash_init.cc (right): https://codereview.chromium.org/2016073004/diff/80001/chrome/browser/ui/ash/a... chrome/browser/ui/ash/ash_init.cc:76: base::WrapUnique(new chromeos::AutoclickRingHandler)); On 2016/06/09 16:41:26, jdufault wrote: > Add trailing () to new AutoclickRingHandler. Done.
Description was changed from ========== Upgrades to auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Show a visual indicator of the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Show a visual indicator of the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Show a visual indicator for the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
https://codereview.chromium.org/2016073004/diff/100001/ash/autoclick/autoclic... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/100001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:63: static const base::TimeDelta kDefaultAutoclickDelay; static member needs to be POD and TimeDelta looks like it's not. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:1: // Copyright (c) 2016 The Chromium Authors. All rights reserved. nit: nuke "(c)". https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:118: widget_->SetAlwaysOnTop(true); always on top container is actually below many windows. You probably want to use OverlayContainer instead. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:120: // We are owned by the AutoclickRing. looks like this is owned by AutoclickRingHandler. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:131: ->ConvertPointToScreen(root_window, &point); isn't the point already in screen coordinates? https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:251: if (!root_window) { can this happen? https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:302: default: case NONE: would be better, and if you add new enum and forgot to handle it, it'll result in compilation error. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:317: default: ditto https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:38: enum AnimationType { optional: enum class https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:58: gfx::Point tap_down_location_; please document the coordinates. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:59: aura::Window* tap_down_target_; please document ownership (or lifetime)
Patchset #6 (id:120001) has been deleted
https://codereview.chromium.org/2016073004/diff/100001/ash/autoclick/autoclic... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/100001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:63: static const base::TimeDelta kDefaultAutoclickDelay; On 2016/06/13 21:57:29, oshima wrote: > static member needs to be POD and TimeDelta looks like it's not. Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:1: // Copyright (c) 2016 The Chromium Authors. All rights reserved. On 2016/06/13 21:57:29, oshima wrote: > nit: nuke "(c)". Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:118: widget_->SetAlwaysOnTop(true); On 2016/06/13 21:57:30, oshima wrote: > always on top container is actually below many windows. You probably want to use > OverlayContainer instead. Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:120: // We are owned by the AutoclickRing. On 2016/06/13 21:57:29, oshima wrote: > looks like this is owned by AutoclickRingHandler. Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:131: ->ConvertPointToScreen(root_window, &point); On 2016/06/13 21:57:29, oshima wrote: > isn't the point already in screen coordinates? Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:251: if (!root_window) { On 2016/06/13 21:57:29, oshima wrote: > can this happen? Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:302: default: On 2016/06/13 21:57:29, oshima wrote: > case NONE: > > would be better, and if you add new enum and forgot to handle it, it'll result > in compilation error. Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:317: default: On 2016/06/13 21:57:30, oshima wrote: > ditto Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:38: enum AnimationType { On 2016/06/13 21:57:30, oshima wrote: > optional: enum class Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:58: gfx::Point tap_down_location_; On 2016/06/13 21:57:30, oshima wrote: > please document the coordinates. Done. https://codereview.chromium.org/2016073004/diff/100001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:59: aura::Window* tap_down_target_; On 2016/06/13 21:57:30, oshima wrote: > please document ownership (or lifetime) Done.
please address comments below, then lgtm https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:27: // Called when an autoclick gesture begins. document the coordinate of |center|. I assume it's screen coordinates? Or you can just use "center_point_in_screen" https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:28: virtual void StartGesture(base::TimeDelta duration, gfx::Point center) = 0; const gfx::Point& https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:35: virtual void SetGestureCenter(gfx::Point center) = 0; ditto https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:63: params.keep_on_top = true; nit: you don't need this. https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:87: } nit: new line https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:89: gfx::Point& center, const gfx::Point& https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:32: void StartGesture(base::TimeDelta duration, gfx::Point center) override; const gfx::Point& https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:58: // Location of the simulated mouse event from auto click. .. in screen coordinates.
sammiequon@chromium.org changed reviewers: + dbeam@chromium.org
@dbeam - please take a look - browser_options.html https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... File ash/autoclick/autoclick_controller.h (right): https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:27: // Called when an autoclick gesture begins. On 2016/06/15 21:26:28, oshima wrote: > document the coordinate of |center|. I assume it's screen coordinates? > > Or you can just use "center_point_in_screen" Done. https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:28: virtual void StartGesture(base::TimeDelta duration, gfx::Point center) = 0; On 2016/06/15 21:26:28, oshima wrote: > const gfx::Point& Done. https://codereview.chromium.org/2016073004/diff/140001/ash/autoclick/autoclic... ash/autoclick/autoclick_controller.h:35: virtual void SetGestureCenter(gfx::Point center) = 0; On 2016/06/15 21:26:28, oshima wrote: > ditto Done. https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.cc (right): https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:63: params.keep_on_top = true; On 2016/06/15 21:26:28, oshima wrote: > nit: you don't need this. Done. https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:87: } On 2016/06/15 21:26:28, oshima wrote: > nit: new line Done. https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.cc:89: gfx::Point& center, On 2016/06/15 21:26:28, oshima wrote: > const gfx::Point& Done. https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... File chrome/browser/chromeos/ui/autoclick_ring_handler.h (right): https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:32: void StartGesture(base::TimeDelta duration, gfx::Point center) override; On 2016/06/15 21:26:28, oshima wrote: > const gfx::Point& Done. https://codereview.chromium.org/2016073004/diff/140001/chrome/browser/chromeo... chrome/browser/chromeos/ui/autoclick_ring_handler.h:58: // Location of the simulated mouse event from auto click. On 2016/06/15 21:26:28, oshima wrote: > .. in screen coordinates. Done.
can you update the Material Design equivalent? i.e. chrome/browser/resources/settings
lgtm
The CQ bit was checked by sammiequon@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org, oshima@chromium.org Link to the patchset: https://codereview.chromium.org/2016073004/#ps220001 (title: "Updated material design equivalent.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
The CQ bit was checked by sammiequon@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by sammiequon@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sammiequon@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dmazzoni@chromium.org, oshima@chromium.org, dbeam@chromium.org Link to the patchset: https://codereview.chromium.org/2016073004/#ps240001 (title: "Rebase.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Show a visual indicator for the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Show a visual indicator for the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #11 (id:240001)
Message was sent while issue was closed.
Description was changed from ========== Show a visual indicator for the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== Show a visual indicator for the progress of auto-click. BUG=610476 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/820646ff0c3aa9fb6ea4d8ff727e9f5b02b1ca2a Cr-Commit-Position: refs/heads/master@{#401913} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/820646ff0c3aa9fb6ea4d8ff727e9f5b02b1ca2a Cr-Commit-Position: refs/heads/master@{#401913} |
