Support decoding GenericNamedValue based policy.

Support decoding GenericNamedValue based policy.

Even if it arrives in the ChromeSettingsProto message where Chrome expects to find explicitly typed policy.
This is necessary for as long as CPanel/D3 deliver old-style policy. We include this fix on the client side to make a corresponding workaround in DMServer unnecessary.


BUG=chromium-os:14102
TEST=UserPolicyCacheTest.OldStylePolicy; manual test against DMServer without the server-side workaround

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=81729

[Jakob Kummerow, 2011-04-13 14:03:52]

PTAL.

[Mattias Nissler (ping if slow), 2011-04-13 17:53:02]

A high-level comment. Also, there seems to be a broken test case.

http://codereview.chromium.org/6840014/diff/1/chrome/browser/policy/user_poli...
File chrome/browser/policy/user_policy_cache.cc (right):

http://codereview.chromium.org/6840014/diff/1/chrome/browser/policy/user_poli...
chrome/browser/policy/user_policy_cache.cc:153:
MaybeDecodeOldstylePolicy(policy, mandatory, recommended);
Instead of hacking the field at position 2 into CloudSettingsProto, why don't we
just have a second proto definition like this:

message LegacyChromeSettingsProto {
  repeated enterprise_management.GenericNamedValue named_value = 2;
}

Then, just do a ParseFromString(policy_data.policy_value()) here and feed the
result into MaybeDecodeOldstyle policy. That way, you can avoid all the code
generation and gyp trickery. Also, removing the code later will be easier.

http://codereview.chromium.org/6840014/diff/1/chrome/browser/policy/user_poli...
chrome/browser/policy/user_policy_cache.cc:175: PolicyMap* policy_map_;
DISALLOW_COPY_AND_ASSIGN

[Jakob Kummerow, 2011-04-14 17:17:00]

http://codereview.chromium.org/6840014/diff/1/chrome/browser/policy/user_poli...
File chrome/browser/policy/user_policy_cache.cc (right):

http://codereview.chromium.org/6840014/diff/1/chrome/browser/policy/user_poli...
chrome/browser/policy/user_policy_cache.cc:153:
MaybeDecodeOldstylePolicy(policy, mandatory, recommended);
On 2011/04/13 17:53:02, Mattias Nissler wrote:
> Instead of hacking the field at position 2 into CloudSettingsProto, why don't
we
> just have a second proto definition like this:
> 
> message LegacyChromeSettingsProto {
>   repeated enterprise_management.GenericNamedValue named_value = 2;
> }
> 
> Then, just do a ParseFromString(policy_data.policy_value()) here and feed the
> result into MaybeDecodeOldstyle policy. That way, you can avoid all the code
> generation and gyp trickery. Also, removing the code later will be easier.

Good idea. Done.

http://codereview.chromium.org/6840014/diff/1/chrome/browser/policy/user_poli...
chrome/browser/policy/user_policy_cache.cc:175: PolicyMap* policy_map_;
On 2011/04/13 17:53:02, Mattias Nissler wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

[Mattias Nissler (ping if slow), 2011-04-14 17:33:45]

LGTM.

Can you make sure we test this against a server to verify?

http://codereview.chromium.org/6840014/diff/11/chrome/browser/policy/user_pol...
File chrome/browser/policy/user_policy_cache.h (right):

http://codereview.chromium.org/6840014/diff/11/chrome/browser/policy/user_pol...
chrome/browser/policy/user_policy_cache.h:44: // The following methods are
needed to support old-style policy and can
overly-nitty nit: In C++, we call them member functions :)