Deprecate unused feature DOMImplementationCreateCSSStyleSheet

Deprecate unused feature DOMImplementationCreateCSSStyleSheet

Intent to Deprecate:
    https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/acGbTAPb4Ls
    
BUG=335001
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166176

[philipj_slow, 2014-01-16 11:24:21]

https://codereview.chromium.org/140743002/diff/1/Source/core/dom/DOMImplement...
File Source/core/dom/DOMImplementation.idl (left):

https://codereview.chromium.org/140743002/diff/1/Source/core/dom/DOMImplement...
Source/core/dom/DOMImplementation.idl:40:
[MeasureAs=DOMImplementationCreateCSSStyleSheet]
This doesn't make sense, if the use counter supports it, we should remove the
feature, not stop measuring it. The first step is to send an "Intent to
Deprecate and Remove", see
http://www.chromium.org/blink#TOC-Launch-Process:-Deprecation

However, this counter hasn't reached the beta channel yet AFAICT, so I'm not
sure if the data collected is sufficient.

https://codereview.chromium.org/140743002/diff/1/Source/core/frame/UseCounter.h
File Source/core/frame/UseCounter.h (right):

https://codereview.chromium.org/140743002/diff/1/Source/core/frame/UseCounter...
Source/core/frame/UseCounter.h:279: DOMImplementationCreateCSSStyleSheet, //
Removed from DOM
Counters that become unused should be replaced by UnusedSlotXX, search in this
file for an example.

[silverroots, 2014-01-16 12:38:20]

Thank you philipj for the review.
I have sent request for Intent to Deprecate and Remove as you suggested.
I will replace the unused counter with UnusedSlot and update the patch.

[esprehn, 2014-01-17 02:53:45]

This patch doesn't make sense, we shouldn't just remove the UseCounter, we
should remove the feature itself.

[silverroots, 2014-01-17 03:01:23]

On 2014/01/17 02:53:45, esprehn wrote:
> This patch doesn't make sense, we shouldn't just remove the UseCounter, we
> should remove the feature itself.

Agree. The feature is unused as well. Not just the counter. This was defined
here -
http://www.w3.org/2003/01/dom2-javadoc/org/w3c/dom/css/DOMImplementationCSS.html
and also Firefox has issues filed here -
https://bugzilla.mozilla.org/show_bug.cgi?id=63850 which also states for not to
implement it. I suppose this can be safely removed.

[philipj_slow, 2014-01-17 03:07:50]

https://codereview.chromium.org/140743002/diff/10003/Source/core/dom/DOMImple...
File Source/core/dom/DOMImplementation.idl (left):

https://codereview.chromium.org/140743002/diff/10003/Source/core/dom/DOMImple...
Source/core/dom/DOMImplementation.idl:40:
[MeasureAs=DOMImplementationCreateCSSStyleSheet]
As per the blink-dev discussion, this should only be deprecated until the use
counter has reached stable, so make this line
[DeprecateAs=DOMImplementationCreateCSSStyleSheet] and add a deprecation message
in UseCounter.cpp.

https://codereview.chromium.org/140743002/diff/10003/Source/core/frame/UseCou...
File Source/core/frame/UseCounter.h (right):

https://codereview.chromium.org/140743002/diff/10003/Source/core/frame/UseCou...
Source/core/frame/UseCounter.h:279: UnusedSlot10, // Removed, was
DOMImplementationCreateCSSStyleSheet.
Revert this change, and note that a CL is on its way to change the convention
from UnusedSlotXX to REMOVEDDOMImplementationCreateCSSStyleSheet:
https://codereview.chromium.org/132983005/

[silverroots, 2014-01-24 11:07:36]

Hi Philipj, 
Can you please review the deprecation patch? Thanks.

[Inactive, 2014-01-24 14:08:48]

Please update your changelog to indicate that you are deprecating the feature,
not removing it.

Also, please add an API Owner as reviewer (e.g. jochen) as web-exposed changes
like this need API Owner approval.

https://codereview.chromium.org/140743002/diff/150001/Source/core/dom/DOMImpl...
File Source/core/dom/DOMImplementation.idl (right):

https://codereview.chromium.org/140743002/diff/150001/Source/core/dom/DOMImpl...
Source/core/dom/DOMImplementation.idl:40:
[DeprecateAs=DOMImplementationCreateCSSStyleSheet]
I believe you also need to add a deprecation message to UseCounter.cpp. Likely,
some layout tests will need rebasing after that as well due to the deprecation
warning.

[philipj_slow, 2014-01-24 14:45:52]

My non-owner review is to concur with what Chris Dumez said :)

https://codereview.chromium.org/140743002/diff/150001/Source/core/dom/DOMImpl...
File Source/core/dom/DOMImplementation.idl (right):

https://codereview.chromium.org/140743002/diff/150001/Source/core/dom/DOMImpl...
Source/core/dom/DOMImplementation.idl:40:
[DeprecateAs=DOMImplementationCreateCSSStyleSheet]
On 2014/01/24 14:08:48, Chris Dumez wrote:
> I believe you also need to add a deprecation message to UseCounter.cpp.
Likely,
> some layout tests will need rebasing after that as well due to the deprecation
> warning.

Indeed, without such a message you will hit an assert. Since there is no
replacement, I can only suggest the somewhat unhelpful deprecation message
"'DOMImplementation.createCSSStyleSheet()' is deprecated."

[silverroots, 2014-01-30 09:01:14]

Hi Chris, Philip,
I have updated the depcreation message and layout tests rebaselined. Please
review. Thanks.

[philipj_slow, 2014-01-30 12:34:03]

On 2014/01/30 09:01:14, silverroots wrote:
> Hi Chris, Philip,
> I have updated the depcreation message and layout tests rebaselined. Please
> review. Thanks.

Non-owner LGTM!

[philipj_slow, 2014-01-30 12:34:22]

On 2014/01/30 09:01:14, silverroots wrote:
> Hi Chris, Philip,
> I have updated the depcreation message and layout tests rebaselined. Please
> review. Thanks.

Non-owner LGTM!

[commit-bot: I haz the power, 2014-01-30 12:55:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaustubh.a@samsung.com/140743002/270001

[commit-bot: I haz the power, 2014-01-30 12:56:31]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-30 12:59:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaustubh.a@samsung.com/140743002/270001

[commit-bot: I haz the power, 2014-01-30 13:09:41]

CQ bit was unchecked on CL. Ignoring.

[jochen (gone - plz use gerrit), 2014-01-30 13:59:28]

lgtm

[Inactive, 2014-01-30 14:30:56]

lgtm

[commit-bot: I haz the power, 2014-01-30 14:31:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaustubh.a@samsung.com/140743002/270001

[commit-bot: I haz the power, 2014-01-30 17:50:08]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[commit-bot: I haz the power, 2014-01-30 17:50:09]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-30 17:50:27]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-30 18:46:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaustubh.a@samsung.com/140743002/270001

[commit-bot: I haz the power, 2014-01-30 21:08:21]

Change committed as 166176

[commit-bot: I haz the power, 2014-01-30 21:08:27]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-30 21:09:43]

CQ bit was unchecked on CL. Ignoring.