Update policy backend and testserver for the newest policy protocol

Update policy backend and testserver for the latest policy protocol


BUG=chromium-os:11253, chromium-os:11254, chromium-os:11255
TEST=DeviceManagementServiceIntegrationTest.WithTestServer

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

[gfeher, 2011-02-21 14:26:22]

This is still based on http://codereview.chromium.org/6520008/, but please take
a high level-review on it.

A problematic point is that timestamps to the server are sent in milliseconds,
but they are always truncated to seconds. This can be fixed but I am not sure if
it's worth the trouble.

[Mattias Nissler (ping if slow), 2011-02-21 14:55:27]

Looks pretty good, just a fee comments.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:84: PersistPolicyTask(
Any reason for the changed indentation?

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:89: : path_(path),
AFAIK, you don't need to increase indentation here.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:332: policy_data.timestamp() /
kPolicyResponseTimestampResolution);
I'd recommend base::Time::UnixEpoch() + base::TimeDelta::FromXXX()

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
File chrome/browser/policy/device_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_policy_identity_strategy.cc:19: #if
defined(OS_CHROMEOS)
Not needed, this file is on compiled on CrOS

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_policy_identity_strategy.cc:49:
chromeos::CrosLibrary::Get()->GetSystemLibrary();
Nice!

But I've heard rumors about the serial number not being available during regular
operation, have you tested this?

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:64: # TODO(gfeher): Remove before
commit.
good idea :)

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:123: if
(self.GetUniqueParam('devicetype') != '2' or
Is that devicetype parameter from a discussion with Hong?

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:320: return (400, 'Invalid
policy_type')
What about user policy? Have I missed something?

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:552: The corresponding device
identifier or None if not found.
You might want to update this comment.

[Jakob Kummerow, 2011-02-21 16:14:59]

I heard you like fixing nits

So I put some comments in your CL so you can fix my nits while you're fixing
Mattias' nits

;-)

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:89: : path_(path),
nit: indentation (":" should be 4 spaces deeper than "PersistPolicyTask(")

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:332: policy_data.timestamp() /
kPolicyResponseTimestampResolution);
On 2011/02/21 14:55:27, Mattias Nissler wrote:
> I'd recommend base::Time::UnixEpoch() + base::TimeDelta::FromXXX()

What? We're not doing any additions here. But we need to convert "milliseconds
since 1970" to "seconds since 1970".

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_controller.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_controller.cc:98: LOG(WARNING) << "More than
one policies in the response of the device "
nit: s/one policies/one policy/

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_controller_unittest.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_controller_unittest.cc:35: explicit
MockCloudPolicyCache(const FilePath& backing_file_path)
I've fixed this in my CL by now, you can remove it here.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
File chrome/browser/policy/device_management_backend_impl.h (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_management_backend_impl.h:1: // Copyright (c) 2010
The Chromium Authors. All rights reserved.
nit: 2011

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
File chrome/browser/policy/device_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_policy_identity_strategy.cc:64: return machine_id_;
s/machine_id_/device_id_/ ?

[gfeher, 2011-02-22 15:57:29]

Thanks for the comments, I've addressed most of them and working on the rest.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:84: PersistPolicyTask(
On 2011/02/21 14:55:27, Mattias Nissler wrote:
> Any reason for the changed indentation?

There must have been, but now I am not seeing it.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:89: : path_(path),
On 2011/02/21 16:15:00, jkummerow wrote:
> nit: indentation (":" should be 4 spaces deeper than "PersistPolicyTask(")

Done.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:332: policy_data.timestamp() /
kPolicyResponseTimestampResolution);
On 2011/02/21 16:15:00, jkummerow wrote:
> On 2011/02/21 14:55:27, Mattias Nissler wrote:
> > I'd recommend base::Time::UnixEpoch() + base::TimeDelta::FromXXX()
> 
> What? We're not doing any additions here. But we need to convert "milliseconds
> since 1970" to "seconds since 1970".

Yep. This is converting seconds to milliseconds.

My note in my first message was about this issue: if it's okay to do so and send
back always milliseconds divisible by 1000 to dmserver or not. I should probably
ask them.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_controller.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_controller.cc:98: LOG(WARNING) << "More than
one policies in the response of the device "
On 2011/02/21 16:15:00, jkummerow wrote:
> nit: s/one policies/one policy/

Done.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_controller_unittest.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_controller_unittest.cc:35: explicit
MockCloudPolicyCache(const FilePath& backing_file_path)
On 2011/02/21 16:15:00, jkummerow wrote:
> I've fixed this in my CL by now, you can remove it here.

Done.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
File chrome/browser/policy/device_management_backend_impl.h (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_management_backend_impl.h:1: // Copyright (c) 2010
The Chromium Authors. All rights reserved.
On 2011/02/21 16:15:00, jkummerow wrote:
> nit: 2011

Done.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
File chrome/browser/policy/device_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_policy_identity_strategy.cc:19: #if
defined(OS_CHROMEOS)
On 2011/02/21 14:55:27, Mattias Nissler wrote:
> Not needed, this file is on compiled on CrOS

Done.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_policy_identity_strategy.cc:49:
chromeos::CrosLibrary::Get()->GetSystemLibrary();
I am still fighting with ChromeOS build to make a working image and test this.
If that won't work I'll remove this.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/device...
chrome/browser/policy/device_policy_identity_strategy.cc:64: return machine_id_;
On 2011/02/21 16:15:00, jkummerow wrote:
> s/machine_id_/device_id_/ ?

Whoops!

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:123: if
(self.GetUniqueParam('devicetype') != '2' or
Maybe. Hong had added some server-side requirements into
device_management_backend.proto and I've just implemented those here.

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:320: return (400, 'Invalid
policy_type')
It is handled in self.ProcessCloudPolicy().

http://codereview.chromium.org/6537020/diff/9025/net/tools/testserver/device_...
net/tools/testserver/device_management.py:552: The corresponding device
identifier or None if not found.
On 2011/02/21 14:55:27, Mattias Nissler wrote:
> You might want to update this comment.

Done.

[Mattias Nissler (ping if slow), 2011-02-22 16:20:50]

Answer the timestamp-related comments.

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/9025/chrome/browser/policy/cloud_...
chrome/browser/policy/cloud_policy_cache.cc:332: policy_data.timestamp() /
kPolicyResponseTimestampResolution);
On 2011/02/22 15:57:29, gfeher wrote:
> On 2011/02/21 16:15:00, jkummerow wrote:
> > On 2011/02/21 14:55:27, Mattias Nissler wrote:
> > > I'd recommend base::Time::UnixEpoch() + base::TimeDelta::FromXXX()
> > 
> > What? We're not doing any additions here. But we need to convert
"milliseconds
> > since 1970" to "seconds since 1970".
> 
> Yep. This is converting seconds to milliseconds.
> 
> My note in my first message was about this issue: if it's okay to do so and
send
> back always milliseconds divisible by 1000 to dmserver or not. I should
probably
> ask them.

What I proposed is just a way of converting from your milliseconds value to a
base::Time value that doesn't require magic constants and knowledge about the
exact definition of time_t. What it does is getting the epoch value (which is 1
Jan 1970 by definition) and adding a TimeDelta, which we initialize using
FromMilliseconds. I find that pretty intuitive.

[gfeher, 2011-02-25 20:36:35]

And now it's ready for review, again!

http://codereview.chromium.org/6537020/diff/24001/net/tools/testserver/device...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/24001/net/tools/testserver/device...
net/tools/testserver/device_management.py:66:
logging.basicConfig(filename=LOG_FILENAME,level=logging.DEBUG)
I didn't forget this!

[Jakob Kummerow, 2011-02-28 11:06:31]

looks pretty good, just a few comments.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_cache.cc:346: LOG(WARNING) << "Failed to
parse CloudPolicySettingsj protobuf.";
nit: j?

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_cache.h (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_cache.h:143: int64
last_policy_server_timestamp_;
Why do we need this as a separate field? As discussed, please use just
|last_policy_refresh_time_|.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_controller_unittest.cc (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_controller_unittest.cc:132:
Return(policy_type));
nit: indentation

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/mock_...
File chrome/browser/policy/mock_device_management_backend.h (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/mock_...
chrome/browser/policy/mock_device_management_backend.h:87:
signed_response.set_timestamp((base::Time::NowFromSystemTime() -
As discussed, please extract a local variable to make the line breaking nicer.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/user_...
File chrome/browser/policy/user_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/user_...
chrome/browser/policy/user_policy_identity_strategy.cc:240: if
(device_token_.empty())
I'd insert curlies here. It's just one line of code, but an additional two lines
of comments.

http://codereview.chromium.org/6537020/diff/24001/net/tools/testserver/device...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/24001/net/tools/testserver/device...
net/tools/testserver/device_management.py:544: }[type],
Not sure if I like this expression. It's concise, yes, but kinda hacky. Also, I
suggest to remove the default value for |type| ("=None"), because this
dictionary lookup would crash in that case.

[gfeher, 2011-02-28 12:21:32]

Thanks, addressed comments.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_cache.cc:346: LOG(WARNING) << "Failed to
parse CloudPolicySettingsj protobuf.";
On 2011/02/28 11:06:31, jkummerow wrote:
> nit: j?

Done.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_cache.h (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_cache.h:143: int64
last_policy_server_timestamp_;
Another reason for using this was that SetUnmanaged() is setting this value to
the current time and we don't want to send that value back to the server.

But now I realize that I can filter is_unamaned_ == true before sending the
value. (And in each case when is_unamanaged_ is possibly set to false, then
last_policy_refresh_time_ is updated with a sensible value.)

In short: Done.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_controller_unittest.cc (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_controller_unittest.cc:132:
Return(policy_type));
On 2011/02/28 11:06:31, jkummerow wrote:
> nit: indentation

Done.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/mock_...
File chrome/browser/policy/mock_device_management_backend.h (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/mock_...
chrome/browser/policy/mock_device_management_backend.h:87:
signed_response.set_timestamp((base::Time::NowFromSystemTime() -
On 2011/02/28 11:06:31, jkummerow wrote:
> As discussed, please extract a local variable to make the line breaking nicer.

Done.

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/user_...
File chrome/browser/policy/user_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/24001/chrome/browser/policy/user_...
chrome/browser/policy/user_policy_identity_strategy.cc:240: if
(device_token_.empty())
On 2011/02/28 11:06:31, jkummerow wrote:
> I'd insert curlies here. It's just one line of code, but an additional two
lines
> of comments.

Done.

http://codereview.chromium.org/6537020/diff/24001/net/tools/testserver/device...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/24001/net/tools/testserver/device...
net/tools/testserver/device_management.py:544: }[type],
How about it now?

[Jakob Kummerow, 2011-02-28 12:32:19]

LGTM (pending trybot happiness).

[gfeher, 2011-02-28 18:56:23]

This is all green now. Mattias please take a look.

[Mattias Nissler (ping if slow), 2011-03-01 10:25:49]

just a couple of nits.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_cache.cc:332: DCHECK(timestamp);
kill the DCHECK, we'll crash in the next line anyway if timestamp is NULL.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_identity_strategy.h (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_identity_strategy.h:47: // Returns the device
ID for this device.
Can we add a sentence here saying how the device ID is different from the
machine ID?

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
File chrome/browser/policy/device_management_backend_impl.cc (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
chrome/browser/policy/device_management_backend_impl.cc:30: const char
DeviceManagementBackendImpl::kValueDeviceType[] = "2";
Why "2"?

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
File chrome/browser/policy/device_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
chrome/browser/policy/device_policy_identity_strategy.cc:26: explicit
OwnershipChecker(DevicePolicyIdentityStrategy* callback)
hm, it's not really a callback, so why not just name it strategy?

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
chrome/browser/policy/device_policy_identity_strategy.cc:35: void Cancel();
You can make use of base::WeakPtr instead, that handles the whole
pointer-reset-on-destruction business for you.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
File chrome/browser/policy/proto/device_management_backend.proto (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
chrome/browser/policy/proto/device_management_backend.proto:254: //   * See
GoogleContentTypeEnum.java
Hm, this is a reference to internal source which is not helpful here, or is it?

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
chrome/browser/policy/proto/device_management_backend.proto:257: //   *
devicetype: MUST BE "1" for Android or "2" for Chrome OS.
Ah, I see where the 1 comes from now :)

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
chrome/browser/policy/proto/device_management_backend.proto:260: //   * agent:
MUST BE no more than 64-char long.
Should we then make sure our agent string will be only 64 chars? Like limiting
it?

http://codereview.chromium.org/6537020/diff/31029/net/tools/testserver/device...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/31029/net/tools/testserver/device...
net/tools/testserver/device_management.py:268: policy =
self._server.policy['google/chromeos/user']['mandatory']
huh? why these changes?

[gfeher, 2011-03-01 15:53:00]

Addressed comments and removed the "ownership-claim" check from
DevicePolicyIdentityStrategy.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_cache.cc (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_cache.cc:332: DCHECK(timestamp);
On 2011/03/01 10:25:49, Mattias Nissler wrote:
> kill the DCHECK, we'll crash in the next line anyway if timestamp is NULL.

Done.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
File chrome/browser/policy/cloud_policy_identity_strategy.h (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/cloud...
chrome/browser/policy/cloud_policy_identity_strategy.h:47: // Returns the device
ID for this device.
On 2011/03/01 10:25:49, Mattias Nissler wrote:
> Can we add a sentence here saying how the device ID is different from the
> machine ID?

Done.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
File chrome/browser/policy/device_management_backend_impl.cc (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
chrome/browser/policy/device_management_backend_impl.cc:30: const char
DeviceManagementBackendImpl::kValueDeviceType[] = "2";
On 2011/03/01 10:25:49, Mattias Nissler wrote:
> Why "2"?

Because the server wants this to be "2".

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
File chrome/browser/policy/device_policy_identity_strategy.cc (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
chrome/browser/policy/device_policy_identity_strategy.cc:26: explicit
OwnershipChecker(DevicePolicyIdentityStrategy* callback)
On 2011/03/01 10:25:49, Mattias Nissler wrote:
> hm, it's not really a callback, so why not just name it strategy?

Done.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/devic...
chrome/browser/policy/device_policy_identity_strategy.cc:35: void Cancel();
On 2011/03/01 10:25:49, Mattias Nissler wrote:
> You can make use of base::WeakPtr instead, that handles the whole
> pointer-reset-on-destruction business for you.

Done.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
File chrome/browser/policy/proto/device_management_backend.proto (right):

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
chrome/browser/policy/proto/device_management_backend.proto:254: //   * See
GoogleContentTypeEnum.java
Then I just remove it.

http://codereview.chromium.org/6537020/diff/31029/chrome/browser/policy/proto...
chrome/browser/policy/proto/device_management_backend.proto:260: //   * agent:
MUST BE no more than 64-char long.
Currently the testserver checks for it and dies in case it's longer.

http://codereview.chromium.org/6537020/diff/31029/net/tools/testserver/device...
File net/tools/testserver/device_management.py (right):

http://codereview.chromium.org/6537020/diff/31029/net/tools/testserver/device...
net/tools/testserver/device_management.py:268: policy =
self._server.policy['google/chromeos/user']['mandatory']
On 2011/03/01 10:25:49, Mattias Nissler wrote:
> huh? why these changes?

Because this is for handling the old protocol and policy scopes do not
correspond to the new policy types. I add back some comments!

[Mattias Nissler (ping if slow), 2011-03-01 16:12:41]

LGTM pending trybots.

[gfeher, 2011-03-01 18:03:52]

As discussed, I've done some last-minute changes.

[Mattias Nissler (ping if slow), 2011-03-01 18:27:11]

still LGTM.