Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
406213002:
If the media controls are visible they should always grab clicks (Closed)
|
Created:
6 years, 5 months ago by aberent Modified:
6 years, 4 months ago CC:
blink-reviews, blink-reviews-html_chromium.org, gasubic, fs, eric.carlson_apple.com, feature-media-reviews_chromium.org, dglazkov+blink, nessy, vcarbune.chromium Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Project:
blink Visibility:
Public. |
DescriptionIf the media controls are visible they should always grab clicks
Once we have decided to show the media controls it makes no sense to
allow the page to change their handling. This CL ensures that, if the
media controls are shown, then clicks on them actually get to them.
BUG=388738
BUG=269454
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179397
Patch Set 1 #Patch Set 2 : Add missing comment #
Total comments: 4
Patch Set 3 : Rewrite, based on philipj@ suggestion to move this to EventPath #Patch Set 4 : Move nodes on which check is done, and add event filters - tests still needed #Patch Set 5 : Test added - now looking for LGTM or detailed comments. #
Total comments: 21
Patch Set 6 : Fix review comments #Patch Set 7 : Fix layout test - was hitting overlay play button. #Messages
Total messages: 41 (0 generated)
This seems a simple fix to this bug, but I don't know enough about the architecture of Blink to know whether this is a reasonable fix or a hack. In particular, is it reasonable to allow MediaControlElements to override preDispatchEventHandler? Please advise.
+acolwell +self
Excellent, this really needs fixing. I've tried to fix issue 269454 a couple of times, but kept running into subtle problems with any preDispatchEventHandler solution. The spec here is "If the user agent exposes a user interface to the user by displaying controls over the media element, then the user agent should suppress any user interaction events while the user agent is interacting with this interface. (For example, if the user clicks on a video's playback control, mousedown events and so forth would not simultaneously be fired at elements on the page.)" In other words, at least mousedown, mouseup, dblclick, keydown, keyup and keypress need to be handled. Even with this patch, the click event isn't being blocked on the time and volume sliders, I think they have an internal shadow DOM which becomes the event target. (EventDispatcher::dispatchEventPreProcess will only call preDispatchEventHandler on the target.) Also, it's still possible to mess with MediaControlTimelineElement by calling preventDefault on mousedown+mouseup and probabably mousemove+input too. I think the ideal behavior would be to block certain events if any visible part of the media controls are clicked, including e.g. the time display and black padding between buttons. I think that determineDispatchBehavior in EventPath.cpp could be an ingredient in making that work more robustly than anything using preDispatchEventHandler. That being said, I don't have an ideal solution ready (or I would have fixed this already) so if no one knows how to fix all of these cases I'm all for fixing the most serious problems first. This will need a test, too. https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElementTypes.cpp (left): https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElementTypes.cpp:84: m_element->setInlineStyleProperty(CSSPropertyDisplay, CSSValueNone); Instead of a flag, hide() and show() could be changed to use setBooleanAttribute(hiddenAttr, true/false) and fastHasAttribute(hiddenAttr) could be used to check the state. https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElementTypes.cpp (right): https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElementTypes.cpp:145: if (event->type() == EventTypeNames::click && !isHidden()) { Why does it actually matter if it's hidden? How are we getting click events at all? Also, extra space before && https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElementTypes.h (left): https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElementTypes.h:120: Accidental removal? https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElementTypes.h (right): https://codereview.chromium.org/406213002/diff/20001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElementTypes.h:126: void * preDispatchEventHandler(Event*) OVERRIDE FINAL; extra space
Patch set 3 is a complete rewrite, based on philipj@'s suggestion that we should do this in EventPath.cpp. I think this solves all cases. Please comment. I still need to add a test, pointers to how to test this sort of thing would be welcome (I haven't done much on Blink, I mainly work on Chrome for Android).
+hayato, who has a lot of recent pathes involving event path.
PS3 looks promising, it now works for the time and volume sliders as well. There are two problems that I can see: 1. All events are blocked, which in particular for mousemove and scroll events may have unintended consequences. 2. Clicking on the time label and in between buttons still fires events. I guess this is tolerable, but filtering in MediaControlPanelElement and MediaControlOverlayPlayButtonElement would be slightly better IMHO. Others may feel differently. I hope that hayato or tkent can speak with authority on the EventPath bits, as I don't have much experience in that area. When it comes to testing, I think your best starting point is LayoutTests/media/media-controls.js. You'll find mediaControlsButton and other helpers. Look for tests that use those. Either pick a few representative buttons to test, or traverse all the children of the Shadow DOM. Send clicks and key events to them, and verify that those don't bubble to the outside, and that capturing event handlers don't see them.
On 2014/07/23 13:48:45, philipj wrote: > PS3 looks promising, it now works for the time and volume sliders as well. There > are two problems that I can see: > > 1. All events are blocked, which in particular for mousemove and scroll events > may have unintended consequences. Yes, I was intending to ask for advice on which events actually should be blocked. Thanks for reminding me. Scroll events, I agree, should not be blocked; however do we not need to suppress mousemove when dragging the volume or timeline sliders? What other events should, or should not, be blocked? Note that, with this structure, we can, if necessary, block different events in different parts of the controls. > > 2. Clicking on the time label and in between buttons still fires events. I guess > this is tolerable, but filtering in MediaControlPanelElement and > MediaControlOverlayPlayButtonElement would be slightly better IMHO. Others may > feel differently. Assuming the other reviewers are happy with the general principles of this patch I can fix this on the next patch. > > I hope that hayato or tkent can speak with authority on the EventPath bits, as I > don't have much experience in that area. > > When it comes to testing, I think your best starting point is > LayoutTests/media/media-controls.js. You'll find mediaControlsButton and other > helpers. Look for tests that use those. Either pick a few representative buttons > to test, or traverse all the children of the Shadow DOM. Send clicks and key > events to them, and verify that those don't bubble to the outside, and that > capturing event handlers don't see them.
On 2014/07/23 13:58:55, aberent wrote: > On 2014/07/23 13:48:45, philipj wrote: > > PS3 looks promising, it now works for the time and volume sliders as well. > There > > are two problems that I can see: > > > > 1. All events are blocked, which in particular for mousemove and scroll events > > may have unintended consequences. > > Yes, I was intending to ask for advice on which events actually should be > blocked. Thanks for reminding me. Scroll events, I agree, should not be blocked; > however do we not need to suppress mousemove when dragging the volume or > timeline sliders? What other events should, or should not, be blocked? Oh right, dragging the sliders. I guess the quickfix is to duplicate the logic for when the mouse* and input events will be handled in MediaControlTimelineElement::defaultEventHandler and prevent those from bubbling. Meh. (A more ambitious project would be to investigate if one could stop using defaultEventHandler() altogether for this stuff, since there are no cases where we actually want the "default behavior" to be cancelable. Plain event handlers which stop propagation when they have handled an event ought to require less duplication, but it'd end up as a head-allocated object like ImageEventListener I think.) As for which events to always block, I think mousedown, mouseup, click, dblclick and the key* events should be a reasonable start. I don't know about the touch events.
Sorry for the late review. The patch looks too hacky to me. I am not pleasant to see introducing exceptional code in EventPath only for MediaControlElement. If my understanding is correct, this patch prevents the page from the getting events, right? We are introducing the behavior which doesn't follow the spec, aren't we?
On 2014/07/29 10:54:16, hayato wrote: > Sorry for the late review. > > The patch looks too hacky to me. I am not pleasant to see introducing > exceptional code in EventPath only for MediaControlElement. > > If my understanding is correct, this patch prevents the page from the getting > events, right? Yes, that is the intention. > We are introducing the behavior which doesn't follow the spec, aren't we? No, in fact (although I didn't know it when I first found the problem) this is making us conformant with the spec. See the comments in https://code.google.com/p/chromium/issues/detail?id=390186, which quote the spec as saying: “If the user agent exposes a user interface to the user by displaying controls over the media element, then the user agent should suppress any user interaction events while the user agent is interacting with this interface. (For example, if the user clicks on a video's playback control, mousedown events and so forth would not simultaneously be fired at elements on the page.)” http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element...
I've built and tested this, the behavior looks correct to me now. With a few nits, the media bits look good to me, so I hope this can be made palatable on the EventPath side as well. acowell, can you PTAL as well? https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... File LayoutTests/media/video-controls-mouse-events-captured.html (right): https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:13: var badEventHandler = function(event) function badEventHandler(event) { } seems more idiomatic, since nothing special is going on here. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:21: There's trailing whitespace here and below. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:49: // Check that the timeline also captures mousemove There's no mousemove event handler registered in the test, so this could regress. https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElements.cpp (right): https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElements.cpp:152: const AtomicString& type = event->type(); This logic is repeated once, so a static bool isUserInteractionEvent() would be nice. https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElements.cpp:438: const AtomicString& type = event->type(); Same here, maybe isUserInteractionEventForSlider()?
Oh right, there's no testing for the keyboard and touch event bits.
On 2014/07/29 11:14:21, aberent wrote: > On 2014/07/29 10:54:16, hayato wrote: > > Sorry for the late review. > > > > The patch looks too hacky to me. I am not pleasant to see introducing > > exceptional code in EventPath only for MediaControlElement. > > > > If my understanding is correct, this patch prevents the page from the getting > > events, right? > > Yes, that is the intention. > > > We are introducing the behavior which doesn't follow the spec, aren't we? > > No, in fact (although I didn't know it when I first found the problem) this is > making us conformant with the spec. See the comments in > https://code.google.com/p/chromium/issues/detail?id=390186, which quote the spec > as saying: > > “If the user agent exposes a user interface to the user by displaying controls > over the media element, then > the user agent should suppress any user interaction events while the user agent > is interacting with this > interface. (For example, if the user clicks on a video's playback control, > mousedown events and so forth > would not simultaneously be fired at elements on the page.)” > > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element... Scanning the standard this is, as far as I can tell, the only case where an attribute should prevent the page from getting the events, so it is not surprising that it requires exceptional code.
https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... File LayoutTests/media/video-controls-mouse-events-captured.html (right): https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:17: video.addEventListener("click", badEventHandler); nit: Use waitForEventAndFail() from video-test.js here and below. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:22: video.addEventListener("loadeddata", function() nit: Use waitForEventOnce() here https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:47: video.addEventListener("mousemove", badEventHandler) ditto https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElements.h (right): https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElements.h:132: virtual bool keepEventInShadowDOM(Event*) OVERRIDE; Perhaps I'm misunderstanding something. Why can't this be placed on MediaControls instead since it is the parent of all the other controls? IIUC we are just trying to prevent events from leaking outside of the all controls right?
On 2014/07/29 11:14:21, aberent wrote: > “If the user agent exposes a user interface to the user by displaying controls > over the media element, then > the user agent should suppress any user interaction events while the user agent > is interacting with this > interface. (For example, if the user clicks on a video's playback control, > mousedown events and so forth > would not simultaneously be fired at elements on the page.)” > > http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element... I got it. Looks exception actually.
https://codereview.chromium.org/406213002/diff/80001/Source/core/events/Event... File Source/core/events/EventPath.cpp (right): https://codereview.chromium.org/406213002/diff/80001/Source/core/events/Event... Source/core/events/EventPath.cpp:129: if (m_event && determineDispatchBehavior(m_event, current, m_node) == StayInsideShadowDOM) After the patch, determineDispatchBehavior will be called every time in this loop. Can we call that only once per TreeScope as before? In the current approach, the parent element of MediaControlElement will be also excluded from the event path even though they are in the same node tree. The spec doesn't say so. All we have to do is prevent events from propagating into parent TreeScope, where the *page* exists, I think.
> In the current approach, the parent element of MediaControlElement will be also > excluded from the event path even though they are in the same node tree. The > spec doesn't say so. All we have to do is prevent events from propagating into > parent TreeScope, where the *page* exists, I think. I misread code. L131 actually checks that. Please ignore the above comment. Can we avoid calling determineDispatchBehavior every time in the loop if we use event.target instead of passing `current` node? In every use cases we have to handle, is event.target set to the MediaControlElement, isn't it?
On 2014/07/30 04:38:11, hayato wrote: > > In the current approach, the parent element of MediaControlElement will be > also > > excluded from the event path even though they are in the same node tree. The > > spec doesn't say so. All we have to do is prevent events from propagating into > > parent TreeScope, where the *page* exists, I think. > > I misread code. L131 actually checks that. Please ignore the above comment. > > Can we avoid calling determineDispatchBehavior every time in the loop if we use > event.target instead of passing `current` node? > In every use cases we have to handle, is event.target set to the > MediaControlElement, isn't it? Unfortunately no. The MediaControlElements themselves have complex structures. If you click on the volume control the target is slider, not the MediaControlVolumeSliderElement element which owns these, and certainly not the MediaControlPanelEnclosureElement. The slider and thumb elements are generic elements, not specific to the media controls. To call determineDispatchBehavior only on the target we would have to create (derive from the standard versions) special versions of all the leaf node types (e.g. sliders) used within the media controls, and give each of them a version of keepEventInShadowDOM(). I have not investigated what else we would need to do to make this work (would we, for example, need new CSS for these elements) but this would certainly add a lot of complexity to the code, and I am against doing it unless we can show that calling determineDispatchBehavior every time we loop gives a measurable performance hit.
On 2014/07/30 08:59:10, aberent wrote: > Unfortunately no. The MediaControlElements themselves have complex structures. > If you click on the volume control the target is slider, not the > MediaControlVolumeSliderElement element which owns these, and certainly not the > MediaControlPanelEnclosureElement. The slider and thumb elements are generic > elements, not specific to the media controls. I got it. I agree that It's unfortunate. It looks we have to call current->keepEventsInShadowDOM for every node in the worst case. How about extract `current->keepEventsInShadowDOM(..)` check from determiniDispatchBehavior(..)? Then, we don't have to call that for every node. I don't have a strong concern about performance here. I would have a clear separation between: - What should be checked once per TreeScope. and - What should be checked in every nodes That would make code more understandable.
On 2014/07/30 09:23:57, hayato wrote: > On 2014/07/30 08:59:10, aberent wrote: > > Unfortunately no. The MediaControlElements themselves have complex structures. > > If you click on the volume control the target is slider, not the > > MediaControlVolumeSliderElement element which owns these, and certainly not > the > > MediaControlPanelEnclosureElement. The slider and thumb elements are generic > > elements, not specific to the media controls. > > I got it. I agree that It's unfortunate. It looks we have to call > current->keepEventsInShadowDOM for every node in the worst case. > > How about extract `current->keepEventsInShadowDOM(..)` check from > determiniDispatchBehavior(..)? > Then, we don't have to call that for every node. > > I don't have a strong concern about performance here. I would have a clear > separation between: > > - What should be checked once per TreeScope. > and > - What should be checked in every nodes > > That would make code more understandable. Yes, also means I can stop at the node that wants to capture the event, and don't have to go up the shadow root. Will be done on next patch (with the name of keepEventsInShadowDOM changed to keepEventInNode).
PTAL - new version uploaded https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... File LayoutTests/media/video-controls-mouse-events-captured.html (right): https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:13: var badEventHandler = function(event) On 2014/07/29 11:31:07, philipj wrote: > function badEventHandler(event) { } seems more idiomatic, since nothing special > is going on here. Done. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:17: video.addEventListener("click", badEventHandler); On 2014/07/29 16:23:26, acolwell wrote: > nit: Use waitForEventAndFail() from video-test.js here and below. Done. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:21: On 2014/07/29 11:31:07, philipj wrote: > There's trailing whitespace here and below. Done. And fixed my Eclipse configuration to do it automatically for JavaScript. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:22: video.addEventListener("loadeddata", function() On 2014/07/29 16:23:26, acolwell wrote: > nit: Use waitForEventOnce() here Actually changed to waitForEventAndEnd(). https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:47: video.addEventListener("mousemove", badEventHandler) On 2014/07/29 16:23:26, acolwell wrote: > ditto Done. https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:49: // Check that the timeline also captures mousemove On 2014/07/29 11:31:07, philipj wrote: > There's no mousemove event handler registered in the test, so this could > regress. I may be missing something here. The line above registers a mousemove event handler. I can't register it at the top because of the initial mousemove into the controls (line 27). https://codereview.chromium.org/406213002/diff/80001/Source/core/events/Event... File Source/core/events/EventPath.cpp (right): https://codereview.chromium.org/406213002/diff/80001/Source/core/events/Event... Source/core/events/EventPath.cpp:129: if (m_event && determineDispatchBehavior(m_event, current, m_node) == StayInsideShadowDOM) On 2014/07/30 03:19:11, hayato wrote: > After the patch, determineDispatchBehavior will be called every time in this > loop. > > Can we call that only once per TreeScope as before? > > In the current approach, the parent element of MediaControlElement will be also > excluded from the event path even though they are in the same node tree. The > spec doesn't say so. All we have to do is prevent events from propagating into > parent TreeScope, where the *page* exists, I think. > > > See discussion on main review thread. https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElements.cpp (right): https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElements.cpp:152: const AtomicString& type = event->type(); On 2014/07/29 11:31:07, philipj wrote: > This logic is repeated once, so a static bool isUserInteractionEvent() would be > nice. Done. https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElements.cpp:438: const AtomicString& type = event->type(); On 2014/07/29 11:31:07, philipj wrote: > Same here, maybe isUserInteractionEventForSlider()? Done. https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... File Source/core/html/shadow/MediaControlElements.h (right): https://codereview.chromium.org/406213002/diff/80001/Source/core/html/shadow/... Source/core/html/shadow/MediaControlElements.h:132: virtual bool keepEventInShadowDOM(Event*) OVERRIDE; On 2014/07/29 16:23:26, acolwell wrote: > Perhaps I'm misunderstanding something. Why can't this be placed on > MediaControls instead since it is the parent of all the other controls? IIUC we > are just trying to prevent events from leaking outside of the all controls > right? Unfortunately, to support the overlay play button, the MediaControls have the same dimensions as the video; so placing this on the MediaControls would prevent the video getting any clicks. We only want to capture clicks on the control bar, not on other parts of the video.
https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... File LayoutTests/media/video-controls-mouse-events-captured.html (right): https://codereview.chromium.org/406213002/diff/80001/LayoutTests/media/video-... LayoutTests/media/video-controls-mouse-events-captured.html:49: // Check that the timeline also captures mousemove On 2014/07/31 10:52:46, aberent wrote: > On 2014/07/29 11:31:07, philipj wrote: > > There's no mousemove event handler registered in the test, so this could > > regress. > > I may be missing something here. The line above registers a mousemove event > handler. I can't register it at the top because of the initial mousemove into > the controls (line 27). Sorry, eye/brain malfunction.
This LGTM now. Thanks for fixing this!
On 2014/07/31 11:26:51, philipj wrote: > This LGTM now. Thanks for fixing this! hayato@, are you now happy with this?
I'm happy with this. :) LGTM.
The CQ bit was checked by aberent@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aberent@chromium.org/406213002/100001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...)
On 2014/08/01 10:07:30, I haz the power (commit-bot) wrote: > Try jobs failed on following builders: > linux_blink_rel on tryserver.blink > (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/1...) The try jobs failed because one of the layout tests was hitting the overlay video play button with the mouse, hence preventing the click from getting to the JS. Fixed by moving where the click happens. Note that I also had to change expected image, since the click no longer starts the video playing.
The CQ bit was checked by aberent@chromium.org
The CQ bit was unchecked by aberent@chromium.org
The CQ bit was checked by aberent@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aberent@chromium.org/406213002/120001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/20109)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)
The CQ bit was checked by aberent@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/aberent@chromium.org/406213002/120001
Message was sent while issue was closed.
Change committed as 179397 |
