Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
222203006:
Prevents double-clicks on a tab close button from aslo maximizing the browser (Closed)
|
Created:
6 years, 8 months ago by jonross Modified:
6 years, 8 months ago CC:
chromium-reviews, kalyank, ben+views_chromium.org, sadrul, ben+ash_chromium.org, tfarina Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionPrevents double-clicks on a tab close button from aslo maximizing the browser
The code responsible for listening to a double-click, and for maximizing the
browser is platform dependant. The path on Aura desktop differs from that of
Chrome OS. In both cases they use alternate means to trigger the maximizing.
This change modifies both so that they only attempt to maximize if the initial
click has the same target as the second click. Tab deletion is destructive by
nature, lead do different targets processing different parts of the same series
of user events.
TEST=WorkspaceEventHandlerTest
BUG=348817
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263841
Patch Set 1 #
Total comments: 26
Patch Set 2 : Add left-click restrictions #
Total comments: 6
Patch Set 3 : Add X11 Unittest #
Total comments: 11
Patch Set 4 : Address bug in toplevel_window_event_handler #
Total comments: 8
Patch Set 5 : #
Total comments: 8
Patch Set 6 : #
Messages
Total messages: 26 (0 generated)
Currently the implementation is separate in both classes. Though I dislike duplicating code, each Aura and Chrome OS provide different related behaviours. I had considered giving both x11_window_event_filter and workspace_event_handler a common ancestor, but wasn't sure of an appropriate location, or how much code saving this would lead to. I've emailed sadrul@ for thoughts on a location to unit test the changes to x11_window_event_filter. It has not test class. The only other x11 unit test is in views_unittests. x11_window_event_filter is used by some of that test suite when running on linux. The unit tests for tabs themselves are not dependent on a browser, so there didn't seem an appropriate place to test the double click behaviour there.
I like the approach. A few small things and questions, thanks for looking into this. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:47: GetNonClientComponent(event->location()); Does this logic allow a right-click to "pollute" the previous_target_component state if it comes first? I am not sure if our logic of detecting double click will make this a non-issue, but I think the xxx_target_component_ variables should only be set for left mouse clicks. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:51: event_target_component_ == previous_target_component) { Once here you probably want to reset the event_target_component_ variable (?) https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:71: if (event->type() == ui::ET_GESTURE_TAP) { I think this method would benefit from early exits. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:88: } else { Should be no need for else after return. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... File ash/wm/workspace/workspace_event_handler_unittest.cc (right): https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:377: EXPECT_TRUE(window_state->IsMaximized()); Does this test still work? Shouldn't it now require actual two clicks on the same component to properly set the previous target component? https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:492: EventConsumingTestWindowDelegate wd; nit: Maybe wd -> delegate, probably in the other methods in this file as well. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:532: // First click will go to a client s/click/tap https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:539: // Second click will go to the header s/click/tap https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right): https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... ui/views/widget/desktop_aura/x11_window_event_filter.cc:116: } Maybe the previous_target_component and mouse_target_component_ should only be set once you make sure it was a left click. https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... ui/views/widget/desktop_aura/x11_window_event_filter.cc:122: previous_target_component == component) { Should mouse_target_component_ variable be reset to HTNOWHERE here? https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... ui/views/widget/desktop_aura/x11_window_event_filter.cc:125: // a minimize/maximize on X11 (Windows converts this to min/max events for s/minimize/restore and s/min/restore?
https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:47: GetNonClientComponent(event->location()); On 2014/04/03 04:13:21, varkha wrote: > Does this logic allow a right-click to "pollute" the previous_target_component > state if it comes first? I am not sure if our logic of detecting double click > will make this a non-issue, but I think the xxx_target_component_ variables > should only be set for left mouse clicks. As long as we're consistent with the use of the variable and comment it appropriately I don't think it matters. Since we're only using it for double clicks I'd be in favor of renaming it to double_click_component_ and only setting it on left clicks. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:88: } else { On 2014/04/03 04:13:21, varkha wrote: > Should be no need for else after return. In fact, explicitly forbidden by style guide: http://www.chromium.org/developers/coding-style#Code_Formatting
Updated to enforce left-click restrictions. Some logic cleanup due to processing events twice. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:47: GetNonClientComponent(event->location()); On 2014/04/03 04:13:21, varkha wrote: > Does this logic allow a right-click to "pollute" the previous_target_component > state if it comes first? I am not sure if our logic of detecting double click > will make this a non-issue, but I think the xxx_target_component_ variables > should only be set for left mouse clicks. Using a mouse for input I wasn't able to get a state where right-click interrupts a double-click action. However if that ever did occur it would break this code. I wrote a unittest that demonstrates this. I'll change this to handle left-mouse only. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:47: GetNonClientComponent(event->location()); On 2014/04/03 04:13:21, varkha wrote: > Does this logic allow a right-click to "pollute" the previous_target_component > state if it comes first? I am not sure if our logic of detecting double click > will make this a non-issue, but I think the xxx_target_component_ variables > should only be set for left mouse clicks. Done. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:47: GetNonClientComponent(event->location()); On 2014/04/04 18:51:45, flackr wrote: > On 2014/04/03 04:13:21, varkha wrote: > > Does this logic allow a right-click to "pollute" the previous_target_component > > state if it comes first? I am not sure if our logic of detecting double click > > will make this a non-issue, but I think the xxx_target_component_ variables > > should only be set for left mouse clicks. > > As long as we're consistent with the use of the variable and comment it > appropriately I don't think it matters. Since we're only using it for double > clicks I'd be in favor of renaming it to double_click_component_ and only > setting it on left clicks. Done. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:51: event_target_component_ == previous_target_component) { On 2014/04/03 04:13:21, varkha wrote: > Once here you probably want to reset the event_target_component_ variable (?) Since WindowEventHandler can receive each event up to two times, this is a good idea. Otherwise two consecutive double-click events would trigger this behaviour. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:71: if (event->type() == ui::ET_GESTURE_TAP) { On 2014/04/03 04:13:21, varkha wrote: > I think this method would benefit from early exits. Done. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler.cc:88: } else { On 2014/04/03 04:13:21, varkha wrote: > Should be no need for else after return. Done. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... File ash/wm/workspace/workspace_event_handler_unittest.cc (right): https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:377: EXPECT_TRUE(window_state->IsMaximized()); On 2014/04/03 04:13:21, varkha wrote: > Does this test still work? Shouldn't it now require actual two clicks on the > same component to properly set the previous target component? It does, but it shouldn't. Closer inspection shows that WindowEventHandler can receive all mouse events up to two times. Depending on the target component, and the rest of the tree. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:492: EventConsumingTestWindowDelegate wd; On 2014/04/03 04:13:21, varkha wrote: > nit: Maybe wd -> delegate, probably in the other methods in this file as well. Done. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:532: // First click will go to a client On 2014/04/03 04:13:21, varkha wrote: > s/click/tap Done. https://codereview.chromium.org/222203006/diff/1/ash/wm/workspace/workspace_e... ash/wm/workspace/workspace_event_handler_unittest.cc:539: // Second click will go to the header On 2014/04/03 04:13:21, varkha wrote: > s/click/tap Done. https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right): https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... ui/views/widget/desktop_aura/x11_window_event_filter.cc:116: } On 2014/04/03 04:13:21, varkha wrote: > Maybe the previous_target_component and mouse_target_component_ should only be > set once you make sure it was a left click. Done. https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... ui/views/widget/desktop_aura/x11_window_event_filter.cc:122: previous_target_component == component) { On 2014/04/03 04:13:21, varkha wrote: > Should mouse_target_component_ variable be reset to HTNOWHERE here? Done. https://codereview.chromium.org/222203006/diff/1/ui/views/widget/desktop_aura... ui/views/widget/desktop_aura/x11_window_event_filter.cc:125: // a minimize/maximize on X11 (Windows converts this to min/max events for On 2014/04/03 04:13:21, varkha wrote: > s/minimize/restore and s/min/restore? Done.
https://codereview.chromium.org/222203006/diff/20001/ash/wm/workspace/workspa... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/20001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:61: double_click_component_ = component; why not only change the double click component if it is not yet a double click (same for double tap). actually if we set the component on mouse released why do we need to set it on mouse pressed? https://codereview.chromium.org/222203006/diff/20001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:64: double_click_component_ == previous_target_component) { i.e. compare component to double click component here https://codereview.chromium.org/222203006/diff/20001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.h (right): https://codereview.chromium.org/222203006/diff/20001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.h:66: int mouse_target_component_; this should have the same name.
Added unit tests for the x11 changes. Also addressed comments. https://codereview.chromium.org/222203006/diff/20001/ash/wm/workspace/workspa... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/20001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:61: double_click_component_ = component; On 2014/04/08 14:08:00, flackr wrote: > why not only change the double click component if it is not yet a double click > (same for double tap). actually if we set the component on mouse released why do > we need to set it on mouse pressed? A nice cleanup vs caching the last one. Will make the change. Alas I need to have the check in both pressed/released as we aren't guaranteed which we receive. https://codereview.chromium.org/222203006/diff/20001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:64: double_click_component_ == previous_target_component) { On 2014/04/08 14:08:00, flackr wrote: > i.e. compare component to double click component here Done. https://codereview.chromium.org/222203006/diff/20001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.h (right): https://codereview.chromium.org/222203006/diff/20001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.h:66: int mouse_target_component_; On 2014/04/08 14:08:00, flackr wrote: > this should have the same name. Done.
Hi Sadrul, Could you be the owner reviewer for this change? It involves the x11_window_event_filter testing that I spoke to you about. Thanks, Jon
https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:45: // and ET_MOUSE_RELEASED events. So it can not get the first released event but still get a double click? This sounds like a bug. https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:48: GetNonClientComponent(event->location()); nit: indent 4 https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:56: GetNonClientComponent(event->location()); nit: move these to scopes where used. https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:68: // WindowEventHandler can receive each event up to two times. Once a May i ask why? If different targeting phases we should only handle the event during one of them. https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:101: if (event->details().tap_count() == 2) { How about an early return when tap count is not yet 2? https://codereview.chromium.org/222203006/diff/40001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right): https://codereview.chromium.org/222203006/diff/40001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.cc:105: int previous_target_component = HTNOWHERE; why do you need this? You can just compare current component with double click component before replacing it?
https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... File ash/wm/workspace/workspace_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:45: // and ET_MOUSE_RELEASED events. On 2014/04/08 21:11:39, flackr wrote: > So it can not get the first released event but still get a double click? This > sounds like a bug. Make a fix at the source: topleve_window_event_handler https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:56: GetNonClientComponent(event->location()); On 2014/04/08 21:11:39, flackr wrote: > nit: move these to scopes where used. Done. https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:68: // WindowEventHandler can receive each event up to two times. Once a On 2014/04/08 21:11:39, flackr wrote: > May i ask why? If different targeting phases we should only handle the event > during one of them. We can see this event during both pre and post dispatch. A restriction to event phase could be added to this event handler/ https://codereview.chromium.org/222203006/diff/40001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.cc:101: if (event->details().tap_count() == 2) { On 2014/04/08 21:11:39, flackr wrote: > How about an early return when tap count is not yet 2? Done. https://codereview.chromium.org/222203006/diff/40001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right): https://codereview.chromium.org/222203006/diff/40001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.cc:105: int previous_target_component = HTNOWHERE; On 2014/04/08 21:11:39, flackr wrote: > why do you need this? You can just compare current component with double click > component before replacing it? It felt cleaner when dealing with early exits. This method may end before doing the current vs previous comparison. So only setting afterwards would miss out on some events. I didn't like the idea of duplicating the setting at each exit.
A couple of nits. ui/views/ LGTM otherwise https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc (right): https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:387: DesktopWindowTreeHost* rwh = static_cast<DesktopWindowTreeHost*>( You shouldn't need the static_cast<> https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:413: DesktopWindowTreeHost* rwh = static_cast<DesktopWindowTreeHost*>( ditto https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:418: native_widget->set_window_component(HTCLIENT); Would a test with both components being non-CLIENT be better? https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.h (right): https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.h:66: int double_click_component_; Call this simply |click_component_|. With 'double' prefix, it reads like the component the double-click happened on, which is not what this tracks.
https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc (right): https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:387: DesktopWindowTreeHost* rwh = static_cast<DesktopWindowTreeHost*>( On 2014/04/10 20:49:59, sadrul wrote: > You shouldn't need the static_cast<> Done. https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:413: DesktopWindowTreeHost* rwh = static_cast<DesktopWindowTreeHost*>( On 2014/04/10 20:49:59, sadrul wrote: > ditto Done. https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/desktop_screen_x11_unittest.cc:418: native_widget->set_window_component(HTCLIENT); On 2014/04/10 20:49:59, sadrul wrote: > Would a test with both components being non-CLIENT be better? The behaviour being tested for should not depend on what is the first target component. However I haven't dug through the layers between the window and the x11_window_event_filter to see if other components are consumed or reacted to. I selected HTCLIENT as that is the target component when the tab close button is clicked on. https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.h (right): https://codereview.chromium.org/222203006/diff/60001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.h:66: int double_click_component_; On 2014/04/10 20:49:59, sadrul wrote: > Call this simply |click_component_|. With 'double' prefix, it reads like the > component the double-click happened on, which is not what this tracks. Done.
lgtm with a nit. https://codereview.chromium.org/222203006/diff/80001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right): https://codereview.chromium.org/222203006/diff/80001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.cc:109: previous_target_component = click_component_; nit: The names should match (and possibly match the name in ash).
lgtm too with nits https://codereview.chromium.org/222203006/diff/80001/ash/wm/toplevel_window_e... File ash/wm/toplevel_window_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/80001/ash/wm/toplevel_window_e... ash/wm/toplevel_window_event_handler.cc:497: // make the event as handled so no other handlers/observers act upon the s/make/mark https://codereview.chromium.org/222203006/diff/80001/ash/wm/toplevel_window_e... ash/wm/toplevel_window_event_handler.cc:499: // of destructive actions such as hidding. They should not act upon them. s/hidding/hiding https://codereview.chromium.org/222203006/diff/80001/ash/wm/workspace/workspa... File ash/wm/workspace/workspace_event_handler.h (right): https://codereview.chromium.org/222203006/diff/80001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.h:44: int double_click_component_; click_component_?
https://codereview.chromium.org/222203006/diff/80001/ash/wm/toplevel_window_e... File ash/wm/toplevel_window_event_handler.cc (right): https://codereview.chromium.org/222203006/diff/80001/ash/wm/toplevel_window_e... ash/wm/toplevel_window_event_handler.cc:497: // make the event as handled so no other handlers/observers act upon the On 2014/04/12 03:27:46, flackr wrote: > s/make/mark Done. https://codereview.chromium.org/222203006/diff/80001/ash/wm/toplevel_window_e... ash/wm/toplevel_window_event_handler.cc:499: // of destructive actions such as hidding. They should not act upon them. On 2014/04/12 03:27:46, flackr wrote: > s/hidding/hiding Done. https://codereview.chromium.org/222203006/diff/80001/ash/wm/workspace/workspa... File ash/wm/workspace/workspace_event_handler.h (right): https://codereview.chromium.org/222203006/diff/80001/ash/wm/workspace/workspa... ash/wm/workspace/workspace_event_handler.h:44: int double_click_component_; On 2014/04/12 03:27:46, flackr wrote: > click_component_? Done. https://codereview.chromium.org/222203006/diff/80001/ui/views/widget/desktop_... File ui/views/widget/desktop_aura/x11_window_event_filter.cc (right): https://codereview.chromium.org/222203006/diff/80001/ui/views/widget/desktop_... ui/views/widget/desktop_aura/x11_window_event_filter.cc:109: previous_target_component = click_component_; On 2014/04/11 19:00:12, varkha wrote: > nit: The names should match (and possibly match the name in ash). Done.
The CQ bit was checked by jonross@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jonross@chromium.org/222203006/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on linux_chromium_clang_dbg tryserver.chromium on linux_chromium_gn_rel
The CQ bit was checked by jonross@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jonross@chromium.org/222203006/100001
The CQ bit was unchecked by commit-bot@chromium.org
The commit queue went berserk retrying too often for a seemingly flaky test on builder android_dbg: http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db... http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
The CQ bit was checked by jonross@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jonross@chromium.org/222203006/100001
Message was sent while issue was closed.
Change committed as 263841
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/239213003/ by bungeman@google.com. The reason for reverting is: Broke build.. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
