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

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment
link

Issue 771283: Remove ImageCapture.setOptions()

Reported by mcasas@chromium.org, Oct 3 2017 Project Member

Issue description

Comment 1 by nikhil.s...@samsung.com, Oct 4 2017

I will try to check this issue.

Comment 2 by mcasas@chromium.org, Oct 4 2017

Cc: guidou@chromium.org mcasas@chromium.org nikhil.s...@samsung.com
Owner: mcasas@chromium.org
Status: Assigned (was: Available)
Assigned to me as placeholder for: nikhil.sahni@samsung.com

Comment 4 by nikhil.s...@samsung.com, Oct 12 2017

Added one more patch for measuring the Usage of ImageCapture.setOptions()

https://chromium-review.googlesource.com/c/chromium/src/+/712135.

Comment 5 by bugdroid1@chromium.org, Oct 13 2017

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e858ba31cf1dd066a9a77ddb5b974e96061b0bf

commit 8e858ba31cf1dd066a9a77ddb5b974e96061b0bf
Author: nikhil <nikhil.sahni@samsung.com>
Date: Fri Oct 13 06:03:28 2017

Measuring ImageCapture.setOptions() usage.

To Remove ImageCapture.setOptions() from idl
as per [1] need to measure the usage.
[1] - https://github.com/w3c/mediacapture-image/pull/150/files#diff-ec9cfa5f3f35ec1f84feb2e59686c34dL54

BUG= 771283 

Change-Id: Iab3b99a43c285db4328fe23d8a2bba8dd1552df3
Reviewed-on: https://chromium-review.googlesource.com/712135
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Cr-Commit-Position: refs/heads/master@{#508613}
[modify] https://crrev.com/8e858ba31cf1dd066a9a77ddb5b974e96061b0bf/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
[modify] https://crrev.com/8e858ba31cf1dd066a9a77ddb5b974e96061b0bf/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/8e858ba31cf1dd066a9a77ddb5b974e96061b0bf/tools/metrics/histograms/enums.xml

Comment 6 by foolip@chromium.org, Oct 13 2017

Labels: Merge-Request-63

Comment 7 by gov...@chromium.org, Oct 13 2017

Cc: abdulsyed@chromium.org
Labels: M-63
Does this need merge to M62? If yes, please request ASAP.

Comment 8 by sheriffbot@chromium.org, Oct 14 2017

Project Member
Labels: -Merge-Request-63 Hotlist-Merge-Approved Merge-Approved-63
Your change meets the bar and is auto-approved for M63. Please go ahead and merge the CL to branch 3239 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by mcasas@chromium.org, Oct 14 2017

Labels: -M-63 M-65
#7: this change is a removal of a user-facing method, so will need
to bake for a while. Removing M-63 label; this will not be merged.

Comment 10 by gov...@chromium.org, Oct 16 2017

M63 merge was requested at comment #6. Per comment #9, this won't be merged to M63. Could you please double confirm and remove "Merge-Approved-63" label if M63 merge is not needed?

Comment 11 by guidou@chromium.org, Oct 16 2017

My understanding is that the merge request to 63 is for r508613, which adds counters, but does not fix the bug.
The reason is that removal of a user-facing method requires approval by Blink API owners, which in turns requires gathering of usage data and it is a good idea to have that data for 63.
foolip@: is that correct?

Comment 12 by gov...@chromium.org, Oct 16 2017

Cc: foolip@chromium.org
+ foolip@, PTAL comment #11. Thank you.

Comment 13 by sheriffbot@chromium.org, Oct 17 2017

Project Member
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 14 by foolip@chromium.org, Oct 17 2017

#11, yes, what I intended was merging the use counters only, nothing else.

Comment 15 by mcasas@chromium.org, Oct 17 2017

Ok, merging 8e858ba31cf1dd066a9a77ddb5b974e96061b0bf to 3239.
It's a bit strange to reuse this bug to merge-back a CL. What
if somewhere down the line we need to merge another CL?

Comment 16 by bugdroid1@chromium.org, Oct 17 2017

Project Member
Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3a35098747842a9ffdcdff8f6679606145dfa434

commit 3a35098747842a9ffdcdff8f6679606145dfa434
Author: Miguel Casas-Sanchez <mcasas@chromium.org>
Date: Tue Oct 17 18:42:59 2017

Measuring ImageCapture.setOptions() usage.

To Remove ImageCapture.setOptions() from idl
as per [1] need to measure the usage.
[1] - https://github.com/w3c/mediacapture-image/pull/150/files#diff-ec9cfa5f3f35ec1f84feb2e59686c34dL54

BUG= 771283 
TBR=nikhil.sahni@samsung.com

(cherry picked from commit 8e858ba31cf1dd066a9a77ddb5b974e96061b0bf)

Change-Id: Iab3b99a43c285db4328fe23d8a2bba8dd1552df3
Reviewed-on: https://chromium-review.googlesource.com/712135
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Cr-Original-Commit-Position: refs/heads/master@{#508613}
Reviewed-on: https://chromium-review.googlesource.com/723168
Cr-Commit-Position: refs/branch-heads/3239@{#33}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/3a35098747842a9ffdcdff8f6679606145dfa434/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
[modify] https://crrev.com/3a35098747842a9ffdcdff8f6679606145dfa434/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/3a35098747842a9ffdcdff8f6679606145dfa434/tools/metrics/histograms/enums.xml

Comment 17 by bugdroid1@chromium.org, Jan 19 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a35f17126683c5270273b4a3c765ad05fb829f35

commit a35f17126683c5270273b4a3c765ad05fb829f35
Author: nikhil <nikhil.sahni@samsung.com>
Date: Fri Jan 19 11:59:22 2018

Remove ImageCapture.setOptions()

Remove ImageCapture.setOptions() from idl
and related code according to [1].

[1] - https://github.com/w3c/mediacapture-image/pull/150/files#diff-ec9cfa5f3f35ec1f84feb2e59686c34dL54

Blink Dev Intent Thread - https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/tPbZ0eaO-yw

BUG= 771283 

Change-Id: I2098dac37f47bdc31840fcd54fe421f7834b5442
Reviewed-on: https://chromium-review.googlesource.com/699959
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: NIKHIL SAHNI <nikhil.sahni@samsung.com>
Cr-Commit-Position: refs/heads/master@{#530488}
[modify] https://crrev.com/a35f17126683c5270273b4a3c765ad05fb829f35/third_party/WebKit/LayoutTests/imagecapture/setOptions-reject.html
[delete] https://crrev.com/b3afb6541c4b3c29f9f7b5797be82c6456da30d2/third_party/WebKit/LayoutTests/imagecapture/setOptions.html
[modify] https://crrev.com/a35f17126683c5270273b4a3c765ad05fb829f35/third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/a35f17126683c5270273b4a3c765ad05fb829f35/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/a35f17126683c5270273b4a3c765ad05fb829f35/third_party/WebKit/Source/modules/imagecapture/ImageCapture.idl
[modify] https://crrev.com/a35f17126683c5270273b4a3c765ad05fb829f35/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/a35f17126683c5270273b4a3c765ad05fb829f35/tools/metrics/histograms/enums.xml

Comment 18 by mcasas@chromium.org, Feb 5 2018

Status: Fixed (was: Assigned)
Fixed by nikhil.sahni@samsung.com with crrev.com/c/699959

Sign in to add a comment