New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 839742 link

Starred by 2 users

Issue metadata

Status: Duplicate
Merged: issue 846379
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: [DevTools] Eye dropper doesn’t appear on moving the mouse pointer out of DevTools.

Reported by dchau...@etouch.net, May 4 2018

Issue description

Chrome Version: 68.0.3419.0 (Official Build)Revision	222512a0ce7deab81dad3af945bad2196e8a8838-refs/heads/master@{#555962} 32/64-bit.
OS: Windows (7,8,8.1,10),Mac OS X(10.12.6,10.13.1,10.13.5)

What steps will reproduce the problem?
1. Launch Chrome, Open DevTools and click to open ColorPicker under under 'Styles' section.
2. Now, move the mouse pointer out of DevTools window and observe.

Actual: Eye dropper doesn’t appear.
Expected: Eye dropper should appear.

This is a regression issue, broken in M-68 series, will soon update other info.
 
Labels: hasbisect-per-revision OS-Linux
Owner: chfremer@chromium.org
Status: Assigned (was: Unconfirmed)
Below is manual regression range for the above issue:

Good build: 68.0.3418.0 (Revision: 555652)
Bad build: 68.0.3419.0 (Revision: 555962)

Using the per-revision bisect, providing the bisect results:

You are probably looking for a change made after 555717 (known good), but no later than 555718 (first known bad).
CHANGELOG URL:
The script might not always return single CL as suspect as some perf builds might get missing due to failure.
  https://chromium.googlesource.com/chromium/src/+log/7e9f7ca863e24f299ddb67f3ec8232a7f55818b4..1cecc366923d3a4ff31f9f36465c7da1155f7a16

Suspecting: https://chromium.googlesource.com/chromium/src/+/1cecc366923d3a4ff31f9f36465c7da1155f7a16

@chfremer: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

NOTE: This issue also reproducible on Linux (14.04 LTS) OS.

Kindly review the attached screen-cast for reference.

Thank you.
Actual behavior.mov
4.5 MB View Download
Expected behavior.mov
2.4 MB View Download
Labels: ReleaseBlock-Stable
marking as RBS, please change if required.
I was able to repro the issue on Linux.
I tried to revert the suspected CL locally and found that this seems to fix the issue.

This is very surprising, because not only does that CL have no (intended) functional change, but actually none of the code that it touches should be even executed for this test case.

Before trying to dig into a possible connection between the CL and the issue, let me double check that this is not due to differences in the active experiments.
Cc: x...@chromium.org m...@chromium.org
+miu@ and xjz@ for visibility

Okay, this is understood now.
In the CL I made the mistake to add a type mapping from media.mojom.VideoCapturePixelFormat to media::VideoPixelFormat that silently interprets everything that is not I420 or Y16 as I420 [1].

viz::FrameSinkVideoCapturerImpl actually uses ARGB, and my type mapping silently mapped this to I420, which causes the issue.

The type mapping was a bad idea. I'll see if I can remove it altogether.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1026795/12/media/capture/mojom/video_capture_types_mojom_traits.cc#121
Friendly ping to get an update on this issue as it is marked as stable blocker.
Please update on the thread as per C#4.

Thank you..!
As owner last visit shown as 17 days ago,could someone from cc'ed dev please take a look into it as per c#4?

Thanks in advance!
Sorry for the delay. My CL for fixing it [1] was still waiting for a sign off when I stepped out for a paternity leave. 

I am back at office starting tomorrow, and will do my best to get it landed ASAP.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=839742#c5
Status: Fixed (was: Assigned)
Some concerns were raised during review of my CL from #7, and it is not clear if or when it will land.

However, a fix has been landed in the meantime by samans@ (Thank you!) , see https://chromium-review.googlesource.com/c/chromium/src/+/1072034.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-68; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-68 label, otherwise remove Merge-TBD label. Thanks.
Mergedinto: 846379
Status: Duplicate (was: Fixed)
A fix has landed in 68.0.3440.0 (see duplicate issue), so merge to M68 should not be needed.
Labels: -Merge-TBD
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4610d04fac766c5f47e51a8294e3a48da94c0190

commit 4610d04fac766c5f47e51a8294e3a48da94c0190
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jun 12 20:09:02 2018

Define enum media.mojom.VideoPixelFormat instead of using native enum

A recent CL https://chromium-review.googlesource.com/c/chromium/src/+/1026795
caused a regression by introducing a new mojo enum VideoCapturePixelType and
typemapping it to media::VideoPixelFormat, see  Bug 839742 .

The motivation for that change was to make the video capture service API,
which had a dependency on the native enum VideoPixelFormat in media_types.mojom,
compatible with an older version of Mojo used on ChromeOS. This older version
does not support native enums.

This CL replaces the bad solution from the CL above with the following:
* Instead of using a native enum VideoPixelFormat in media_types.mojom, declare
  a Mojo enum and move it to a separate file.
* Add typmapping from the Mojo enum to the native enum.
* Have the video capture service API mojoms depend on only the new file
  containing the Mojo enum for VideoPixelFormat. This way we avoid the API
  depending on any file that uses native enums.

Bug:  839742 ,  847598 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ifcff064b1f0eb95579119916b757dc6512f49a15
Reviewed-on: https://chromium-review.googlesource.com/1050489
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566543}
[modify] https://crrev.com/4610d04fac766c5f47e51a8294e3a48da94c0190/media/capture/mojom/video_capture_types.mojom
[modify] https://crrev.com/4610d04fac766c5f47e51a8294e3a48da94c0190/media/capture/mojom/video_capture_types_mojom_traits.cc
[modify] https://crrev.com/4610d04fac766c5f47e51a8294e3a48da94c0190/media/mojo/interfaces/video_decoder_config_struct_traits.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ceadf1def1186c0db6a9a47ccbf04054593c4d0

commit 6ceadf1def1186c0db6a9a47ccbf04054593c4d0
Author: Christian Fremerey <chfremer@chromium.org>
Date: Tue Jun 12 22:15:54 2018

Revert "Define enum media.mojom.VideoPixelFormat instead of using native enum"

This reverts commit 4610d04fac766c5f47e51a8294e3a48da94c0190.

Reason for revert: Forgot to update the CL description. Will reland to avoid a quite confusing/misleading CL description.

Original change's description:
> Define enum media.mojom.VideoPixelFormat instead of using native enum
> 
> A recent CL https://chromium-review.googlesource.com/c/chromium/src/+/1026795
> caused a regression by introducing a new mojo enum VideoCapturePixelType and
> typemapping it to media::VideoPixelFormat, see  Bug 839742 .
> 
> The motivation for that change was to make the video capture service API,
> which had a dependency on the native enum VideoPixelFormat in media_types.mojom,
> compatible with an older version of Mojo used on ChromeOS. This older version
> does not support native enums.
> 
> This CL replaces the bad solution from the CL above with the following:
> * Instead of using a native enum VideoPixelFormat in media_types.mojom, declare
>   a Mojo enum and move it to a separate file.
> * Add typmapping from the Mojo enum to the native enum.
> * Have the video capture service API mojoms depend on only the new file
>   containing the Mojo enum for VideoPixelFormat. This way we avoid the API
>   depending on any file that uses native enums.
> 
> Bug:  839742 ,  847598 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: Ifcff064b1f0eb95579119916b757dc6512f49a15
> Reviewed-on: https://chromium-review.googlesource.com/1050489
> Commit-Queue: Christian Fremerey <chfremer@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#566543}

TBR=xhwang@chromium.org,rockot@chromium.org,sandersd@chromium.org,rsesek@chromium.org,chfremer@chromium.org,lasoren@chromium.org

Change-Id: Iab9f2c00a6e5aa8ffd167b4d89f294691b37d456
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  839742 ,  847598 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1097900
Reviewed-by: Christian Fremerey <chfremer@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566601}
[modify] https://crrev.com/6ceadf1def1186c0db6a9a47ccbf04054593c4d0/media/capture/mojom/video_capture_types.mojom
[modify] https://crrev.com/6ceadf1def1186c0db6a9a47ccbf04054593c4d0/media/capture/mojom/video_capture_types_mojom_traits.cc
[modify] https://crrev.com/6ceadf1def1186c0db6a9a47ccbf04054593c4d0/media/mojo/interfaces/video_decoder_config_struct_traits.cc

Sign in to add a comment