|
|
Created:
6 years, 3 months ago by Mayur Kankanwadi Modified:
6 years, 2 months ago CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, sof, eae+blinkwatch, rwlbuis Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Project:
blink Visibility:
Public. |
DescriptionNotify in console when an API fails because it needs to be invoked on user action.
This CL shows a console warning on requestFullScreen, requestPointerLock and
Notification.requestPermission api's, when they are not invoked on user action.
BUG=365779
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183069
Patch Set 1 #
Total comments: 8
Patch Set 2 : Review comments added with reset -expected.txt files. #
Total comments: 8
Patch Set 3 : Fixed console message and rebased expected.txt files. #
Total comments: 7
Patch Set 4 : Using ExceptionMessage for generating msgs. Updated expected.txt files. #Patch Set 5 : Removed requestPointerLock related changes. #
Messages
Total messages: 36 (5 generated)
mayurk.vk@samsung.com changed reviewers: + felt@chromium.org, meacer@chromium.org
Hi All, This is first cut of the fix. This is breaking 17 layout tests, due to the console warnings. If the fix is ok, then I will upload the rebaselined -expected.txt files. Thanks. --Mayur Kankanwadi.
peter@chromium.org changed reviewers: + peter@chromium.org
https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... File Source/modules/notifications/Notification.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... Source/modules/notifications/Notification.cpp:167: } We removed the user gesture requirement for requesting Web Notification permissions in M36. Are you still seeing this?
https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... File Source/modules/notifications/Notification.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... Source/modules/notifications/Notification.cpp:167: } On 2014/09/24 14:59:44, Peter Beverloo wrote: > We removed the user gesture requirement for requesting Web Notification > permissions in M36. Are you still seeing this? No, not seeing any permission dialog for notifications. But the issue here was to make the user/developer aware of such a thing happening.
https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... File Source/modules/notifications/Notification.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... Source/modules/notifications/Notification.cpp:167: } On 2014/09/24 15:03:42, Mayur Kankanwadi wrote: > On 2014/09/24 14:59:44, Peter Beverloo wrote: > > We removed the user gesture requirement for requesting Web Notification > > permissions in M36. Are you still seeing this? > No, not seeing any permission dialog for notifications. But the issue here was > to make the user/developer aware of such a thing happening. I made a test-page here: http://peter.sh/files/notification.html The request comes through just fine. We actually allowed this because us requiring the gesture was not in line with the specification. Why should we warn in this case?
Thanks for taking over this bug! I added some comments. https://codereview.chromium.org/600773002/diff/1/Source/core/dom/Fullscreen.cpp File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/core/dom/Fullscreen.c... Source/core/dom/Fullscreen.cpp:236: "Permission denied for requestFullScreen() as it was invoked by script, without any user interaction involved!")); The message sounds a bit verbose. You might want to make this look like similar messages: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/ex... "requestFullScreen can only be initiated by a user gesture" https://codereview.chromium.org/600773002/diff/1/Source/core/page/PointerLock... File Source/core/page/PointerLockController.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/core/page/PointerLock... Source/core/page/PointerLockController.cpp:54: "requestPointerLock() was invoked by script, without any user interaction involved!Press 'ESC' to unlock the pointer.")); Same here as requestFullScreen https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... File Source/modules/notifications/Notification.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... Source/modules/notifications/Notification.cpp:167: } On 2014/09/24 17:14:20, Peter Beverloo wrote: > On 2014/09/24 15:03:42, Mayur Kankanwadi wrote: > > On 2014/09/24 14:59:44, Peter Beverloo wrote: > > > We removed the user gesture requirement for requesting Web Notification > > > permissions in M36. Are you still seeing this? > > No, not seeing any permission dialog for notifications. But the issue here was > > to make the user/developer aware of such a thing happening. > > I made a test-page here: > http://peter.sh/files/notification.html > > The request comes through just fine. We actually allowed this because us > requiring the gesture was not in line with the specification. Why should we warn > in this case? I agree, a gesture isn't required for this API so no need to warn for it.
Thanks for the review. Uploaded new patch. PTAL. https://codereview.chromium.org/600773002/diff/1/Source/core/dom/Fullscreen.cpp File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/core/dom/Fullscreen.c... Source/core/dom/Fullscreen.cpp:236: "Permission denied for requestFullScreen() as it was invoked by script, without any user interaction involved!")); On 2014/09/24 17:59:21, Mustafa Emre Acer wrote: > The message sounds a bit verbose. You might want to make this look like similar > messages: > https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/ex... > > "requestFullScreen can only be initiated by a user gesture" Done. https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... File Source/modules/notifications/Notification.cpp (right): https://codereview.chromium.org/600773002/diff/1/Source/modules/notifications... Source/modules/notifications/Notification.cpp:167: } On 2014/09/24 17:59:21, Mustafa Emre Acer wrote: > On 2014/09/24 17:14:20, Peter Beverloo wrote: > > On 2014/09/24 15:03:42, Mayur Kankanwadi wrote: > > > On 2014/09/24 14:59:44, Peter Beverloo wrote: > > > > We removed the user gesture requirement for requesting Web Notification > > > > permissions in M36. Are you still seeing this? > > > No, not seeing any permission dialog for notifications. But the issue here > was > > > to make the user/developer aware of such a thing happening. > > > > I made a test-page here: > > http://peter.sh/files/notification.html > > > > The request comes through just fine. We actually allowed this because us > > requiring the gesture was not in line with the specification. Why should we > warn > > in this case? > > I agree, a gesture isn't required for this API so no need to warn for it. Done.
https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated by a user gesture.")); Should we save this string "can only be initiated by a user gesture" somewhere for reuse with the same string, if/when it needs to be added for other APIs? That way we don't end up with different strings in different places.
https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated by a user gesture.")); On 2014/09/25 08:48:39, felt wrote: > Should we save this string "can only be initiated by a user gesture" somewhere > for reuse with the same string, if/when it needs to be added for other APIs? > That way we don't end up with different strings in different places. Though I haven't noticed this usage pattern before anywhere in Blink/Webkit, we can probably give it a shot. Just a simple search for throwDOMException shows the amount of string duplication within Blink. We can target this as a separate bug, where in we replace duplicated exception or console log strings with AtomicStrings. Does it sound good?
https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated by a user gesture.")); On 2014/09/25 08:59:07, Mayur Kankanwadi wrote: > On 2014/09/25 08:48:39, felt wrote: > > Should we save this string "can only be initiated by a user gesture" somewhere > > for reuse with the same string, if/when it needs to be added for other APIs? > > That way we don't end up with different strings in different places. > Though I haven't noticed this usage pattern before anywhere in Blink/Webkit, we > can probably give it a shot. > Just a simple search for throwDOMException shows the amount of string > duplication within Blink. > We can target this as a separate bug, where in we replace duplicated exception > or console log strings with AtomicStrings. > Does it sound good? Fine with me.
On 2014/09/26 05:35:46, felt wrote: > https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... > File Source/core/dom/Fullscreen.cpp (right): > > https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... > Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated by > a user gesture.")); > On 2014/09/25 08:59:07, Mayur Kankanwadi wrote: > > On 2014/09/25 08:48:39, felt wrote: > > > Should we save this string "can only be initiated by a user gesture" > somewhere > > > for reuse with the same string, if/when it needs to be added for other APIs? > > > That way we don't end up with different strings in different places. > > Though I haven't noticed this usage pattern before anywhere in Blink/Webkit, > we > > can probably give it a shot. > > Just a simple search for throwDOMException shows the amount of string > > duplication within Blink. > > We can target this as a separate bug, where in we replace duplicated exception > > or console log strings with AtomicStrings. > > Does it sound good? > > Fine with me. I created a new issue: crbug.com/418008 : Provide a message/exception string formatting class in Blink similar to one in v8. peter, felt, meacer : Can you please share your thoughts on the bug? Thanks.
On 2014/09/26 07:16:52, Mayur Kankanwadi wrote: > On 2014/09/26 05:35:46, felt wrote: > > > https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... > > File Source/core/dom/Fullscreen.cpp (right): > > > > > https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... > > Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated > by > > a user gesture.")); > > On 2014/09/25 08:59:07, Mayur Kankanwadi wrote: > > > On 2014/09/25 08:48:39, felt wrote: > > > > Should we save this string "can only be initiated by a user gesture" > > somewhere > > > > for reuse with the same string, if/when it needs to be added for other > APIs? > > > > That way we don't end up with different strings in different places. > > > Though I haven't noticed this usage pattern before anywhere in Blink/Webkit, > > we > > > can probably give it a shot. > > > Just a simple search for throwDOMException shows the amount of string > > > duplication within Blink. > > > We can target this as a separate bug, where in we replace duplicated > exception > > > or console log strings with AtomicStrings. > > > Does it sound good? > > > > Fine with me. > > I created a new issue: > crbug.com/418008 : Provide a message/exception string formatting class in Blink > similar to one in v8. > peter, felt, meacer : Can you please share your thoughts on the bug? > Thanks. Also if everything is fine with the solution, can I expect a LGTM?
Lgtm with a few comments. https://codereview.chromium.org/600773002/diff/20001/LayoutTests/fullscreen/f... File LayoutTests/fullscreen/full-screen-request-rejected-expected.txt (right): https://codereview.chromium.org/600773002/diff/20001/LayoutTests/fullscreen/f... LayoutTests/fullscreen/full-screen-request-rejected-expected.txt:1: CONSOLE WARNING: requestFullScreen() can only be intiated by a user gesture. intiated -> initiated https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated by a user gesture.")); intiated -> initiated I don't think you need parenthesis either. "requestFullScreen can only be initiated by a user gesture" looks clearer. https://codereview.chromium.org/600773002/diff/20001/Source/core/page/Pointer... File Source/core/page/PointerLockController.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:54: "requestPointerLock() can only be intiated by a user gesture.")); Same here, no need for paranthesis and intiated -> initiated "requestPointerLock can only be initiated by a user gesture."
PTAL. Changed the console message as per the suggestions. Thanks. https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen() can only be intiated by a user gesture.")); On 2014/09/26 17:47:38, Mustafa Emre Acer wrote: > intiated -> initiated > > I don't think you need parenthesis either. > "requestFullScreen can only be initiated by a user gesture" looks clearer. Done. https://codereview.chromium.org/600773002/diff/20001/Source/core/page/Pointer... File Source/core/page/PointerLockController.cpp (right): https://codereview.chromium.org/600773002/diff/20001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:54: "requestPointerLock() can only be intiated by a user gesture.")); On 2014/09/26 17:47:38, Mustafa Emre Acer wrote: > Same here, no need for paranthesis and intiated -> initiated > > "requestPointerLock can only be initiated by a user gesture." Done.
mayurk.vk@samsung.com changed reviewers: + abarth@chromium.org
PTAL.
mkwst@chromium.org changed reviewers: + mkwst@chromium.org
Two drive-by comments. https://codereview.chromium.org/600773002/diff/40001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/40001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen can only be initiated by a user gesture.")); I'd suggest pulling in 'ExceptionMessages::failedToExecute' in order to maintain consistency in our error messages. https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... File Source/core/page/PointerLockController.cpp (right): https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:54: "requestPointerLock can only be initiated by a user gesture.")); Same here. https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:55: } You should return early here, right? Otherwise you're not actually blocking the execution.
Hi All, As per Mike West's suggestion, updated the code to use ExceptionMessages for generating the message dumped on the console. PTAL. Thanks. https://codereview.chromium.org/600773002/diff/40001/Source/core/dom/Fullscre... File Source/core/dom/Fullscreen.cpp (right): https://codereview.chromium.org/600773002/diff/40001/Source/core/dom/Fullscre... Source/core/dom/Fullscreen.cpp:236: "requestFullScreen can only be initiated by a user gesture.")); On 2014/09/29 12:24:08, Mike West wrote: > I'd suggest pulling in 'ExceptionMessages::failedToExecute' in order to maintain > consistency in our error messages. Done. https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... File Source/core/page/PointerLockController.cpp (right): https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:54: "requestPointerLock can only be initiated by a user gesture.")); On 2014/09/29 12:24:08, Mike West wrote: > Same here. Done. https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:55: } On 2014/09/29 12:24:08, Mike West wrote: > You should return early here, right? Otherwise you're not actually blocking the > execution. The existing code flow does not do anything noticing the request was not initiated by user gesture. We are just doing a check for posting the console message. There is no intention to change the existing flow in anyway.
https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... File Source/core/page/PointerLockController.cpp (right): https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... Source/core/page/PointerLockController.cpp:55: } On 2014/09/30 13:56:40, Mayur Kankanwadi wrote: > On 2014/09/29 12:24:08, Mike West wrote: > > You should return early here, right? Otherwise you're not actually blocking > the > > execution. > > The existing code flow does not do anything noticing the request was not > initiated by user gesture. > We are just doing a check for posting the console message. There is no intention > to change the existing flow in anyway. Then I don't understand why you're adding a message. You're saying "Hey, you can only do this in the context of a user gesture!" but then you let the execution continue. Why?
On 2014/09/30 14:18:12, Mike West wrote: > https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... > File Source/core/page/PointerLockController.cpp (right): > > https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... > Source/core/page/PointerLockController.cpp:55: } > On 2014/09/30 13:56:40, Mayur Kankanwadi wrote: > > On 2014/09/29 12:24:08, Mike West wrote: > > > You should return early here, right? Otherwise you're not actually blocking > > the > > > execution. > > > > The existing code flow does not do anything noticing the request was not > > initiated by user gesture. > > We are just doing a check for posting the console message. There is no > intention > > to change the existing flow in anyway. > > Then I don't understand why you're adding a message. You're saying "Hey, you can > only do this in the context of a user gesture!" but then you let the execution > continue. > > Why? If the developer is using the requestfullscreen/pointerlock api's in scripts which are not invoked by a user gesture(like a timeout), then currently there is no indication that the api expects a user gesture to proceed. This console log will inform the developer about the misstep.
On 2014/09/30 14:28:29, Mayur Kankanwadi wrote: > On 2014/09/30 14:18:12, Mike West wrote: > > > https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... > > File Source/core/page/PointerLockController.cpp (right): > > > > > https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... > > Source/core/page/PointerLockController.cpp:55: } > > On 2014/09/30 13:56:40, Mayur Kankanwadi wrote: > > > On 2014/09/29 12:24:08, Mike West wrote: > > > > You should return early here, right? Otherwise you're not actually > blocking > > > the > > > > execution. > > > > > > The existing code flow does not do anything noticing the request was not > > > initiated by user gesture. > > > We are just doing a check for posting the console message. There is no > > intention > > > to change the existing flow in anyway. > > > > Then I don't understand why you're adding a message. You're saying "Hey, you > can > > only do this in the context of a user gesture!" but then you let the execution > > continue. > > > > Why? > > If the developer is using the requestfullscreen/pointerlock api's in scripts > which are not invoked by a user gesture(like a timeout), > then currently there is no indication that the api expects a user gesture to > proceed. > This console log will inform the developer about the misstep. OK. Where is that check done? I didn't see it, skimming this file.
On 2014/09/30 14:30:31, Mike West wrote: > On 2014/09/30 14:28:29, Mayur Kankanwadi wrote: > > On 2014/09/30 14:18:12, Mike West wrote: > > > > > > https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... > > > File Source/core/page/PointerLockController.cpp (right): > > > > > > > > > https://codereview.chromium.org/600773002/diff/40001/Source/core/page/Pointer... > > > Source/core/page/PointerLockController.cpp:55: } > > > On 2014/09/30 13:56:40, Mayur Kankanwadi wrote: > > > > On 2014/09/29 12:24:08, Mike West wrote: > > > > > You should return early here, right? Otherwise you're not actually > > blocking > > > > the > > > > > execution. > > > > > > > > The existing code flow does not do anything noticing the request was not > > > > initiated by user gesture. > > > > We are just doing a check for posting the console message. There is no > > > intention > > > > to change the existing flow in anyway. > > > > > > Then I don't understand why you're adding a message. You're saying "Hey, you > > can > > > only do this in the context of a user gesture!" but then you let the > execution > > > continue. > > > > > > Why? > > > > If the developer is using the requestfullscreen/pointerlock api's in scripts > > which are not invoked by a user gesture(like a timeout), > > then currently there is no indication that the api expects a user gesture to > > proceed. > > This console log will inform the developer about the misstep. > > OK. Where is that check done? I didn't see it, skimming this file. UserGestureIndicator::processingUserGesture() provides us the information of whether the api was invoked due to user gesture or not. The check is on PointerLockController.cpp:52 & Fullscreen.cpp:234.
> UserGestureIndicator::processingUserGesture() provides us the information of > whether > the api was invoked due to user gesture or not. > The check is on PointerLockController.cpp:52 & Fullscreen.cpp:234. Either I'm missing your point, or you're missing mine. :) The check on line 52 allows you to output this error message. It does not, so far as I can tell, actually stop the execution of the method. Is there another spot in the flow of execution which prevents pointerlock if there's no user gesture? If not, I don't understand why we're adding a message. If so, then I missed it. :)
Let's try one more time. :) >Is there another > spot in the flow of execution which prevents pointerlock if there's no user > gesture? Yes, an ipc message is then sent to the browser process, which does the checks and does not show the usual requestpermission dialog, as the api is not invoked on a user gesture. The idea here is not to stop the execution, but to reveal the reason for it's non-functioning behavior to the end-user. In FF, if we try to call mozRequestFullScreen on a timeout/onload, the following message is logged in console: -- Request for full-screen was denied because Element.mozRequestFullScreen() was not called from inside a short running user-generated event handler -- Hence the bug(crbug.com/365779) logged was to add a similar message in Chrome to let the user know why the "request" did not do anything. I have added a test page(https://code.google.com/p/chromium/issues/detail?id=365779#c11), which when loaded in FF should show the console message. PTAL. Thanks.
On 2014/10/01 06:32:57, Mayur Kankanwadi wrote: > Yes, an ipc message is then sent to the browser process, which does the checks > and does not show the usual requestpermission dialog, as the api is not invoked > on a user gesture. Ok, great! I don't understand why we send the IPC to the browser at all, however, if we know here that the call is going to fail. I'd suggest that this case should be implemented in the same way as the sandboxing check directly below: if we know that the document is sandboxed, we queue up an error event, and return right away. I agree 100% that we should add a console message. That part of the change LGTM. I just don't think that waiting until we hit the browser to block execution is the right thing to do.
On 2014/10/01 08:43:04, Mike West wrote: > I don't understand why we send the IPC to the browser at all, however, if we > know here that the call is going to fail. I'd suggest that this case should be > implemented in the same way as the sandboxing check directly below: if we know > that the document is sandboxed, we queue up an error event, and return right > away. Yes, the spec mentions that the pointerlockerror event must be fired if for some "technical" reason the permission is denied(https://developer.mozilla.org/en-US/docs/Web/Events/pointerlockerror). I assume the user gesture requirement failure is a "technical" reason. We probably need to address this in a separate issue/bug. felt,meacer,mike: Should the requestpointerlock console message from this CL?
On 2014/10/01 09:19:11, MayurK (OOO 4D starting..) wrote: > On 2014/10/01 08:43:04, Mike West wrote: > > I don't understand why we send the IPC to the browser at all, however, if we > > know here that the call is going to fail. I'd suggest that this case should be > > implemented in the same way as the sandboxing check directly below: if we know > > that the document is sandboxed, we queue up an error event, and return right > > away. > Yes, the spec mentions that the pointerlockerror event must be fired if for some > "technical" > reason the permission is > denied(https://developer.mozilla.org/en-US/docs/Web/Events/pointerlockerror). > I assume the user gesture requirement failure is a "technical" reason. > We probably need to address this in a separate issue/bug. > > felt,meacer,mike: Should the requestpointerlock console message from this CL? Edit:felt,meacer,mike: Should the requestpointerlock console message be removed from this CL?
I'd suggest limiting this CL to the Fullscreen API, and creating another CL that fires the error event, etc. for PointerLock.
(The Fullscreen portions of this CL LGTM, by the way. :) )
On 2014/10/01 12:32:58, Mike West wrote: > (The Fullscreen portions of this CL LGTM, by the way. :) ) Thanks.
The CQ bit was checked by mayurk.vk@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/600773002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as 183069 |