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

Issue 771283 link

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

Remove ImageCapture.setOptions()

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

Issue description

I will try to check this issue.
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
Added one more patch for measuring the Usage of ImageCapture.setOptions()

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


Project Member

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

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.
Project Member

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

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.
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?
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?
Cc: foolip@chromium.org
+ foolip@, PTAL comment #11. Thank you.
Project Member

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

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
#11, yes, what I intended was merging the use counters only, nothing else.
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?
Project Member

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

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

Project Member

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

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

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

Sign in to add a comment