Add a policy to re-enable deprecated web platform features.

Add a policy to re-enable deprecated web platform features.

This will give admins the ability to re-enable deprecated features
for a limited amount of time.

BUG=

[Mattias Nissler (ping if slow), 2014-05-06 14:31:15]

Here's a first stab at the CL that adds the policy knob to enable deprecated web
platform features. Please take a look  - I've put some comments and questions
inline.

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
File chrome/browser/policy/configuration_policy_handler_list_factory.cc (right):

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:499:
{"ShowModalDialog",
We should talk some more about the naming - do we want to explicitly include the
intended removal date into the tag name?

We could as well put a hard time limit here, and we'd automatically stop
supporting the feature once we're past that time. That would have the benefit of
avoiding the negative effect of people staying on old versions just because
those still support the deprecation policy - the feature would self destruct at
a prescribed point in time. WDYT?

https://codereview.chromium.org/263423003/diff/1/content/public/common/render...
File content/public/common/renderer_preferences.h (right):

https://codereview.chromium.org/263423003/diff/1/content/public/common/render...
content/public/common/renderer_preferences.h:158: std::vector<bool>
deprecated_features_to_enable;
Not sure whether it's better to put this here or in WebPreferences. Opinions?

[Andrew T Wilson (Slow), 2014-05-08 11:32:45]

lgtm with some comments

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
File chrome/browser/policy/configuration_policy_handler_list_factory.cc (right):

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:497:
StringToIntEnumListPolicyHandler::MappingEntry
BTW, should add a TODO + a crbug entry to remove this once the deprecation date
is passed.

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:499:
{"ShowModalDialog",
On 2014/05/06 14:31:15, Mattias Nissler wrote:
> We should talk some more about the naming - do we want to explicitly include
the
> intended removal date into the tag name?
> 
> We could as well put a hard time limit here, and we'd automatically stop
> supporting the feature once we're past that time. That would have the benefit
of
> avoiding the negative effect of people staying on old versions just because
> those still support the deprecation policy - the feature would self destruct
at
> a prescribed point in time. WDYT?

If people are going to stay on old versions to keep deprecated features, then
they'll just stick to a version that is pre-deprecation.

That said, I like the idea of hardcoding a deprecation date, so I'm OK with
doing that.

Also, I think we should absolutely come up with some standard for naming that
includes the final deprecation date - I'd suggest "XXXXUntilDate" (for example:
"ShowModalDialogUntilDec2014").

https://codereview.chromium.org/263423003/diff/1/chrome/browser/renderer_pref...
File chrome/browser/renderer_preferences_util.cc (right):

https://codereview.chromium.org/263423003/diff/1/chrome/browser/renderer_pref...
chrome/browser/renderer_preferences_util.cc:98: value <
content::RENDERER_PREFERENCES_DEPRECATED_FEATURE_COUNT) {
Hmmm. Should the bounds check be a DCHECK instead?

https://codereview.chromium.org/263423003/diff/1/chrome/test/data/policy/poli...
File chrome/test/data/policy/policy_test_cases.json (right):

https://codereview.chromium.org/263423003/diff/1/chrome/test/data/policy/poli...
chrome/test/data/policy/policy_test_cases.json:2082: "os": ["win", "linux",
"mac", "chromeos"],
Why not also android?

https://codereview.chromium.org/263423003/diff/1/components/policy/resources/...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/263423003/diff/1/components/policy/resources/...
components/policy/resources/policy_templates.json:6388: - ShowModalDialog
Should update this to include an expiration date as we previously discussed

[Mattias Nissler (ping if slow), 2014-05-09 16:38:26]

Eric, Adam, can one of you check whether this works for you? Note there are some
questions in patch set 1.

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
File chrome/browser/policy/configuration_policy_handler_list_factory.cc (right):

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:497:
StringToIntEnumListPolicyHandler::MappingEntry
On 2014/05/08 11:32:45, Andrew T Wilson wrote:
> BTW, should add a TODO + a crbug entry to remove this once the deprecation
date
> is passed.

Adam, would you like to track this as part of http://crbug.com/345831 or would
you prefer a separate bug?

https://codereview.chromium.org/263423003/diff/1/chrome/browser/policy/config...
chrome/browser/policy/configuration_policy_handler_list_factory.cc:499:
{"ShowModalDialog",
On 2014/05/08 11:32:45, Andrew T Wilson wrote:
> On 2014/05/06 14:31:15, Mattias Nissler wrote:
> > We should talk some more about the naming - do we want to explicitly include
> the
> > intended removal date into the tag name?
> > 
> > We could as well put a hard time limit here, and we'd automatically stop
> > supporting the feature once we're past that time. That would have the
benefit
> of
> > avoiding the negative effect of people staying on old versions just because
> > those still support the deprecation policy - the feature would self destruct
> at
> > a prescribed point in time. WDYT?
> 
> If people are going to stay on old versions to keep deprecated features, then
> they'll just stick to a version that is pre-deprecation.

That's certainly a risk, but they'd have to retain an old copy of the installer
and wipe Chrome state for all users etc., so that's pretty painful...

In the end, we need to decide which option we think is less risky overall. I
tend to lean towards timed self-destruct. What do others think?

> 
> That said, I like the idea of hardcoding a deprecation date, so I'm OK with
> doing that.
> 
> Also, I think we should absolutely come up with some standard for naming that
> includes the final deprecation date - I'd suggest "XXXXUntilDate" (for
example:
> "ShowModalDialogUntilDec2014").

Okay, how about <FEATURE>_Until_<yyyymmdd>? If we do self-destruct, we should
specify the day IMHO.

https://codereview.chromium.org/263423003/diff/1/chrome/browser/renderer_pref...
File chrome/browser/renderer_preferences_util.cc (right):

https://codereview.chromium.org/263423003/diff/1/chrome/browser/renderer_pref...
chrome/browser/renderer_preferences_util.cc:98: value <
content::RENDERER_PREFERENCES_DEPRECATED_FEATURE_COUNT) {
On 2014/05/08 11:32:45, Andrew T Wilson wrote:
> Hmmm. Should the bounds check be a DCHECK instead?

Well, pref values are not guaranteed to be totally under Chrome's control in
general, so I prefer this version to make absolutely sure we don't write out of
bounds.

https://codereview.chromium.org/263423003/diff/1/chrome/test/data/policy/poli...
File chrome/test/data/policy/policy_test_cases.json (right):

https://codereview.chromium.org/263423003/diff/1/chrome/test/data/policy/poli...
chrome/test/data/policy/policy_test_cases.json:2082: "os": ["win", "linux",
"mac", "chromeos"],
On 2014/05/08 11:32:45, Andrew T Wilson wrote:
> Why not also android?

Good point, fixed.

[abarth-chromium, 2014-05-10 00:27:39]

I'm not very familiar with this side of things, so I don't have much technical
feedback on the CL, but I answered the two questions I found.  Please let me
know if I missed any questions.

https://codereview.chromium.org/263423003/diff/1/components/policy/resources/...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/263423003/diff/1/components/policy/resources/...
components/policy/resources/policy_templates.json:6388: - ShowModalDialog
On 2014/05/08 11:32:45, Andrew T Wilson wrote:
> Should update this to include an expiration date as we previously discussed

Yeah, I think that's a good idea to make sure admins know how long they've got.

https://codereview.chromium.org/263423003/diff/1/content/public/common/render...
File content/public/common/renderer_preferences.h (right):

https://codereview.chromium.org/263423003/diff/1/content/public/common/render...
content/public/common/renderer_preferences.h:158: std::vector<bool>
deprecated_features_to_enable;
On 2014/05/06 14:31:15, Mattias Nissler wrote:
> Not sure whether it's better to put this here or in WebPreferences. Opinions?

These are going to map down to WebRuntimeFeatures, which I think we typically
represent using WebPreferences.

[Mattias Nissler (ping if slow), 2014-05-19 15:03:08]

Drew, PTAL.

https://codereview.chromium.org/263423003/diff/1/components/policy/resources/...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/263423003/diff/1/components/policy/resources/...
components/policy/resources/policy_templates.json:6388: - ShowModalDialog
On 2014/05/10 00:27:39, abarth wrote:
> On 2014/05/08 11:32:45, Andrew T Wilson wrote:
> > Should update this to include an expiration date as we previously discussed
> 
> Yeah, I think that's a good idea to make sure admins know how long they've
got.

Done.

https://codereview.chromium.org/263423003/diff/1/content/public/common/render...
File content/public/common/renderer_preferences.h (right):

https://codereview.chromium.org/263423003/diff/1/content/public/common/render...
content/public/common/renderer_preferences.h:158: std::vector<bool>
deprecated_features_to_enable;
On 2014/05/10 00:27:39, abarth wrote:
> On 2014/05/06 14:31:15, Mattias Nissler wrote:
> > Not sure whether it's better to put this here or in WebPreferences.
Opinions?
> 
> These are going to map down to WebRuntimeFeatures, which I think we typically
> represent using WebPreferences.

OK, moved to WebPreferences.

[Mattias Nissler (ping if slow), 2014-05-23 06:52:07]

Drew: friendly ping.

[Andrew T Wilson (Slow), 2014-05-23 15:20:54]

still lgtm

https://codereview.chromium.org/263423003/diff/40001/chrome/browser/chrome_co...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/263423003/diff/40001/chrome/browser/chrome_co...
chrome/browser/chrome_content_browser_client.cc:246: using base::FileDescriptor;
Ah, the tyranny of alphabetical order.

[Mattias Nissler (ping if slow), 2014-05-23 17:11:36]

Adding more OWNERS for various pieces:

darin@chromium.org: webpreferences.h
jar@chromium.org: histograms.xml
dconnelly@chromium.org: policy_templates.json
inferno@chromium.org: common_param_trait_macros.h

[jar (doing other things), 2014-05-23 17:18:44]

histograms.xml LGTM

[eseidel, 2014-05-23 19:32:30]

lgtm

[dconnelly, 2014-05-26 08:32:21]

lgtm

[Andrew T Wilson (Slow), 2014-06-02 12:20:30]

darin/inferno: ping - I'm taking over this CL for Mattias because he's out on
paternity leave.

[Andrew T Wilson (Slow), 2014-06-06 15:10:10]

Jochen: This was that ShowModalDialog-related CL we were discussing.

darin/inferno: PTAL still, or let me know if there's someone else who should.

[jochen (gone - plz use gerrit), 2014-06-10 10:25:23]

On 2014/06/06 15:10:10, Andrew T Wilson wrote:
> Jochen: This was that ShowModalDialog-related CL we were discussing.
> 
> darin/inferno: PTAL still, or let me know if there's someone else who should.

I'm not sure about the layering. I would probably mark the feature as "testing"
in RuntimeFeatures.in (or introduce deprecated as an alias for testing) and then
turn the features on in the chrome content browser client ( instead of adding
this deprecated but on set) wdyt?

[Andrew T Wilson (Slow), 2014-06-10 17:16:25]

jln/tsepez - can one of you give an OWNERS stamp for
common_param_traits_macros.h?

jamesr: OWNERS for webpreferences.h?

In the interest of landing this for M37 so we can deprecate ShowModalDialog(),
I'm going to land as-is and I can work with Jochen to address his review
feedback when we're both back in the office post-branch.

[Tom Sepez, 2014-06-10 17:20:16]

Messages LGTM.

[jamesr, 2014-06-10 17:59:19]

This looks like a complicated mechanism to pipe a single bool through - why not
just pass a bool?  If/when we need a general mechanism we can add it, but this
is overkill.

https://codereview.chromium.org/263423003/diff/60001/webkit/common/webprefere...
File webkit/common/webpreferences.h (right):

https://codereview.chromium.org/263423003/diff/60001/webkit/common/webprefere...
webkit/common/webpreferences.h:170: // A bitset indicating which deprecated
features to enable in the renderer.
the comment says bitset but the type isn't - it's a vector of bools. a single
bool should be sufficient here - why complicate things? i really hope we don't
use this mechanism more generally

[jamesr, 2014-06-10 18:11:27]

https://codereview.chromium.org/263423003/diff/60001/chrome/browser/chrome_co...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/263423003/diff/60001/chrome/browser/chrome_co...
chrome/browser/chrome_content_browser_client.cc:2401:
web_prefs->deprecated_features_to_enable[value] = true;
I understand you're in a hurry to land but this is a crazy amount of code to add
to pipe a single boolean value through.  I'm guessing this is why our size graph
looks like this:

https://chromeperf.appspot.com/report?tests=sizes%2Fchrome&masters=Chromium&b...

Do we really need a generalized mechanism here?  What else do we expect to use
this functionality?

[Andrew T Wilson (Slow), 2014-06-10 18:53:33]

https://codereview.chromium.org/263423003/diff/60001/chrome/browser/chrome_co...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/263423003/diff/60001/chrome/browser/chrome_co...
chrome/browser/chrome_content_browser_client.cc:2401:
web_prefs->deprecated_features_to_enable[value] = true;
On 2014/06/10 18:11:27, jamesr wrote:
> I understand you're in a hurry to land but this is a crazy amount of code to
add
> to pipe a single boolean value through.  I'm guessing this is why our size
graph
> looks like this:
> 
>
https://chromeperf.appspot.com/report?tests=sizes%2Fchrome&masters=Chromium&b...
> 
> Do we really need a generalized mechanism here?  What else do we expect to use
> this functionality?

The goal is that many future deprecated APIs would have associated policy flags
to temporarily extend their life for enterprise users. So a more general policy
framework is required so we don't have to re-roll individual booleans for every
API.

Agreed that this is overkill if we never have another deprecated API that we
want to extend the life, but already we've had a few requests for this and we
expect this to be an ongoing issue.

[jamesr, 2014-06-10 19:16:04]

This is definitely overkill for one bool and may or may not be overkill in the
future, depending on what happens.  Let's see if we need this.  If you want to
keep the goop up in chrome, you can at least map to a single bool before going
to WebPreferences and hitting the IPC channel.