DevTools for Android Doesn't Work With Viz |
||||||||||||||||||
Issue description1) PageHandler Supports remote debugging by casting screenshots of the page on Android to the hosting devtools on desktop. A while ago we added support for DevTools remote debugging in Viz. This is in PageHandler::OnFrameFromVideoConsumer However at the time Viz on Android wasn't far enough along for this to become the system-wide default. So all of this is not compiled in Android. 2) DevToolsEyeDropper This was updated to make use of viz::ClientFrameSinkVideoCapturer however the legacy path is still around. This leaves DevTools as the last consumer of WebContentsObserver::DidReceiveCompositorFrame. samans@ you worked on DevToolsEyeDropper, and in general on Viz Video Capture. Is this something that is ready for use on Android?
,
Jul 19
I was just trying this with yesterdays ToT as I wanted to delete the legacy listener. A few extra tests: 1) PageHandler does work on Android if I remove the compilation guards. But I would like to hear if samans@ has any concerns with the capture before landing it. 2) Eye Dropper does not seem to work when remote debugging an Android Device in Viz.
,
Jul 19
+kylechar@, +ericrk@
,
Jul 20
,
Jul 20
OK good news. Eye dropper doesn't even work without viz so no fix needed there. Performance timeline already works with viz to my surprise. Only screen-casting doesn't work which seems to be easily fixable by removing compile guards for android.
,
Jul 20
jonross: By the way let's not remove the old code path until M68 is shipped to stable with the new code path.
,
Jul 20
We also seem to miss top controls height and ratio but they can be easily added to VideoFrameMetadata.
,
Jul 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2e7b3e635af6d9fcf7928d30f6887c9d3fae692 commit b2e7b3e635af6d9fcf7928d30f6887c9d3fae692 Author: Saman Sami <samans@chromium.org> Date: Sat Jul 21 00:48:09 2018 Fix screencasting on Android with Viz enabled Use video capture for screencasting. Also ship top controls height and shown ratio. Bug: 865430 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: I6cc7b945fb2937a6552c33ca8b5be10b566a48e1 Reviewed-on: https://chromium-review.googlesource.com/1145231 Reviewed-by: Saman Sami <samans@chromium.org> Reviewed-by: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/heads/master@{#577048} [modify] https://crrev.com/b2e7b3e635af6d9fcf7928d30f6887c9d3fae692/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc [modify] https://crrev.com/b2e7b3e635af6d9fcf7928d30f6887c9d3fae692/content/browser/devtools/protocol/page_handler.cc [modify] https://crrev.com/b2e7b3e635af6d9fcf7928d30f6887c9d3fae692/content/browser/devtools/protocol/page_handler.h [modify] https://crrev.com/b2e7b3e635af6d9fcf7928d30f6887c9d3fae692/media/base/video_frame_metadata.h
,
Jul 22
,
Jul 23
The patch in #8 fixes screencasting. However it doesn't touch on the eyedropper. samans@ noted that it appears that the eyedropper doesn't work for remoting to Android even without Viz. Which seems bad since it can be accessed easily. I'm not sure if if worked before any of our work in the devtools area. Did we want to continue to track that portion here, or should we generate a new issue to forward to devtools owners?
,
Jul 23
I believe that should be a separate bug. Our main goal is to reach performance parity with non-viz, not to introduce new features in DevTools. I don't think the eyedropper is easily accessible actually. We have two choices: either inspect the pixels of the video frames (which have low quality), or somehow ship higher quality images from the remote device, which doesn't sound like a trivial task if one is not familiar with DevTools.
,
Jul 24
,
Jul 27
Seems like when Viz is disabled we receive snapshots both from CopyFromSurface and the video capturer. I'm gonna have to fix this and also verify that screencasting still works on Android WebView.
,
Jul 28
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4e57b6ba91b30a73648619653c94a6768e015cd7 commit 4e57b6ba91b30a73648619653c94a6768e015cd7 Author: Saman Sami <samans@chromium.org> Date: Tue Jul 31 15:42:31 2018 Fix screencasting on Android with Viz enabled Use video capture for screencasting. Also ship top controls height and shown ratio. Bug: 865430 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: I6cc7b945fb2937a6552c33ca8b5be10b566a48e1 Reviewed-on: https://chromium-review.googlesource.com/1145231 Reviewed-by: Saman Sami <samans@chromium.org> Reviewed-by: Fady Samuel <fsamuel@chromium.org> Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#577048}(cherry picked from commit b2e7b3e635af6d9fcf7928d30f6887c9d3fae692) Reviewed-on: https://chromium-review.googlesource.com/1156565 Cr-Commit-Position: refs/branch-heads/3497@{#272} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/4e57b6ba91b30a73648619653c94a6768e015cd7/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc [modify] https://crrev.com/4e57b6ba91b30a73648619653c94a6768e015cd7/content/browser/devtools/protocol/page_handler.cc [modify] https://crrev.com/4e57b6ba91b30a73648619653c94a6768e015cd7/content/browser/devtools/protocol/page_handler.h [modify] https://crrev.com/4e57b6ba91b30a73648619653c94a6768e015cd7/media/base/video_frame_metadata.h
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f623d0050021d59066c8cc9a828f5b51dddec522 commit f623d0050021d59066c8cc9a828f5b51dddec522 Author: Saman Sami <samans@chromium.org> Date: Tue Jul 31 17:54:58 2018 Don't call CopyFromSurface when using video capture for screencasting Early-out in OnSwapCompositorFrame if video_consumer_ exists. Don't do this in OnSynchronousSwapCompositorFrame because video capture doesn't work with Android WebView and we still have to call CopyFromSurface. Bug: 865430 Change-Id: I4b59848b2c288fc06708187b8d48e86c1fd19237 Reviewed-on: https://chromium-review.googlesource.com/1156871 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/heads/master@{#579473} [modify] https://crrev.com/f623d0050021d59066c8cc9a828f5b51dddec522/content/browser/devtools/protocol/page_handler.cc
,
Jul 31
Requesting to merge #16 into M69. This is a simple change that prevents DevTools from capturing every frame twice.
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/93b0efcef471fa5de61ce8d478b7928d9cc96cdc commit 93b0efcef471fa5de61ce8d478b7928d9cc96cdc Author: Saman Sami <samans@chromium.org> Date: Wed Aug 01 14:45:36 2018 Don't instantiate video_consumer_ in PageHandler on Android WebView Video capture doesn't work on Android WebView, so don't instantiate video_consumer_ in PageHandler Bug: 813929 , 865430 Change-Id: I09ecce34f1f2a50d23fc12e0907a0d27bf3bc9d8 Reviewed-on: https://chromium-review.googlesource.com/1157206 Commit-Queue: Saman Sami <samans@chromium.org> Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Cr-Commit-Position: refs/heads/master@{#579798} [modify] https://crrev.com/93b0efcef471fa5de61ce8d478b7928d9cc96cdc/content/browser/devtools/protocol/page_handler.cc
,
Aug 1
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a33caf38e8281a1f616669c3d41c1bc106597962 commit a33caf38e8281a1f616669c3d41c1bc106597962 Author: Saman Sami <samans@chromium.org> Date: Wed Aug 01 18:50:54 2018 Don't call CopyFromSurface when using video capture for screencasting Early-out in OnSwapCompositorFrame if video_consumer_ exists. Don't do this in OnSynchronousSwapCompositorFrame because video capture doesn't work with Android WebView and we still have to call CopyFromSurface. Bug: 865430 Change-Id: I4b59848b2c288fc06708187b8d48e86c1fd19237 Reviewed-on: https://chromium-review.googlesource.com/1156871 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#579473}(cherry picked from commit f623d0050021d59066c8cc9a828f5b51dddec522) Reviewed-on: https://chromium-review.googlesource.com/1159141 Reviewed-by: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#317} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/a33caf38e8281a1f616669c3d41c1bc106597962/content/browser/devtools/protocol/page_handler.cc
,
Aug 2
,
Aug 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd7b545a89f8bf643883709bcd05ce1fb5aedd94 commit cd7b545a89f8bf643883709bcd05ce1fb5aedd94 Author: Saman Sami <samans@chromium.org> Date: Sat Aug 04 17:14:27 2018 Fix performance timeline screenshots on Android WebView They just don't work at all right now. Video capture doesn't work on Android WebView. We need to call CopyFromSurface. Bug: 813929 , 865430 Change-Id: I4360d3f53f852eb7e3182ac58540a1500cc6fee3 Reviewed-on: https://chromium-review.googlesource.com/1159199 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/heads/master@{#580759} [modify] https://crrev.com/cd7b545a89f8bf643883709bcd05ce1fb5aedd94/content/browser/devtools/protocol/tracing_handler.cc [modify] https://crrev.com/cd7b545a89f8bf643883709bcd05ce1fb5aedd94/content/browser/devtools/render_frame_devtools_agent_host.cc
,
Aug 4
Requesting to merge #22 into M69. This unbreaks an important feature in DevTools that currently doesn't work at all.
,
Aug 4
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 8
Merge approved for M69 branch 3497.
,
Aug 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/18d95c0176dcd4b9c835328c6c175fc854bb282b commit 18d95c0176dcd4b9c835328c6c175fc854bb282b Author: Saman Sami <samans@chromium.org> Date: Wed Aug 08 18:11:11 2018 Fix performance timeline screenshots on Android WebView They just don't work at all right now. Video capture doesn't work on Android WebView. We need to call CopyFromSurface. Bug: 813929 , 865430 Change-Id: I4360d3f53f852eb7e3182ac58540a1500cc6fee3 Reviewed-on: https://chromium-review.googlesource.com/1159199 Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#580759}(cherry picked from commit cd7b545a89f8bf643883709bcd05ce1fb5aedd94) Reviewed-on: https://chromium-review.googlesource.com/1167763 Reviewed-by: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#503} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/18d95c0176dcd4b9c835328c6c175fc854bb282b/content/browser/devtools/protocol/tracing_handler.cc [modify] https://crrev.com/18d95c0176dcd4b9c835328c6c175fc854bb282b/content/browser/devtools/render_frame_devtools_agent_host.cc
,
Aug 9
|
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by fsam...@chromium.org
, Jul 19Status: Assigned (was: Untriaged)