Added a SchemaMap.

Added a SchemaMap.

A Schema defines the expected types of policy values. A SchemaMap maps an
identifier to its Schema. This identifier indicates which component is the
Schema for (e.g. the browser, or an extension).

This class will supersede the PolicyDomainDescriptor, which will be removed.

This class is refcounted so that any thread can work with the schemas (i.e.
a background worker can use Schema data to load the corresponding policies),
and is immutable. Updated version must make a copy of the map, modify it, and
be published as a new SchemaMap. This is handled by a subsequent component.

BUG=270667
R=bartfab@chromium.org, dconnelly@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233597

[Joao da Silva, 2013-10-29 20:29:49]

@Bartosz: please review

@Daniel: FYI

Thanks!

[Joao da Silva, 2013-10-30 11:03:39]

@Bartosz: please review. Thanks!

[bartfab (slow), 2013-10-30 12:34:49]

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
File chrome/browser/policy/schema_map.h (right):

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:19: typedef std::map<std::string, Schema>
ComponentMap;
Is a forward-declaration of Schema really sufficient for a map typedef?

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:32: const ComponentMap&
GetComponents(PolicyDomain domain) const;
Could this return a pointer instead, eliminating the need for 
kEmptyComponentMap_?

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:34: const Schema* GetSchema(PolicyDomain
domain,
Why this accessor? Would it not be cleaner to have all callers construct a
PolicyNamespace when they want to get a schema?

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:51: DISALLOW_COPY_AND_ASSIGN(SchemaMap);
#include "base/basictypes.h"

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
File chrome/browser/policy/schema_map_unittest.cc (right):

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map_unittest.cc:72:
EXPECT_TRUE(map->GetSchema(POLICY_DOMAIN_CHROME, "") == NULL);
Why not:

EXPECT_FALSE(map->GetSchema(POLICY_DOMAIN_CHROME, ""));

[dconnelly, 2013-10-31 10:04:16]

lgtm

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
File chrome/browser/policy/schema_map_unittest.cc (right):

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map_unittest.cc:52: return NULL;
this shouldn't ever happen, right? why not ASSERT_TRUE(schema.valid())?

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
File components/policy/core/common/schema.h (right):

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
components/policy/core/common/schema.h:60: bool Matches(const base::Value&
value) const;
nit: I think the word "validate" is the generally accepted verb for checking a
value against a schema. At least this is true in the XML world.

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
File components/policy/core/common/schema_unittest.cc (right):

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
components/policy/core/common/schema_unittest.cc:18: OBJECT_TYPE ","
I know this is just moved, but why is this necessary?  The other types haven't
been lifted into constants...

[Joao da Silva, 2013-10-31 13:34:55]

Bartosz: PTAL

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
File chrome/browser/policy/schema_map.h (right):

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:19: typedef std::map<std::string, Schema>
ComponentMap;
On 2013/10/30 12:34:49, bartfab wrote:
> Is a forward-declaration of Schema really sufficient for a map typedef?

It is, but this could be confusing for users of this header. Done.

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:32: const ComponentMap&
GetComponents(PolicyDomain domain) const;
On 2013/10/30 12:34:49, bartfab wrote:
> Could this return a pointer instead, eliminating the need for 
> kEmptyComponentMap_?

Done.

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:34: const Schema* GetSchema(PolicyDomain
domain,
On 2013/10/30 12:34:49, bartfab wrote:
> Why this accessor? Would it not be cleaner to have all callers construct a
> PolicyNamespace when they want to get a schema?

Done.

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map.h:51: DISALLOW_COPY_AND_ASSIGN(SchemaMap);
On 2013/10/30 12:34:49, bartfab wrote:
> #include "base/basictypes.h"

Done.

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
File chrome/browser/policy/schema_map_unittest.cc (right):

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map_unittest.cc:52: return NULL;
On 2013/10/31 10:04:16, dconnelly wrote:
> this shouldn't ever happen, right? why not ASSERT_TRUE(schema.valid())?

Because ASSERT_* is a glorified return, and doesn't work in functions that don't
return void.

https://codereview.chromium.org/50143010/diff/1/chrome/browser/policy/schema_...
chrome/browser/policy/schema_map_unittest.cc:72:
EXPECT_TRUE(map->GetSchema(POLICY_DOMAIN_CHROME, "") == NULL);
On 2013/10/30 12:34:49, bartfab wrote:
> Why not:
> 
> EXPECT_FALSE(map->GetSchema(POLICY_DOMAIN_CHROME, ""));

Done.

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
File components/policy/core/common/schema.h (right):

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
components/policy/core/common/schema.h:60: bool Matches(const base::Value&
value) const;
On 2013/10/31 10:04:16, dconnelly wrote:
> nit: I think the word "validate" is the generally accepted verb for checking a
> value against a schema. At least this is true in the XML world.

Done.

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
File components/policy/core/common/schema_unittest.cc (right):

https://codereview.chromium.org/50143010/diff/1/components/policy/core/common...
components/policy/core/common/schema_unittest.cc:18: OBJECT_TYPE ","
On 2013/10/31 10:04:16, dconnelly wrote:
> I know this is just moved, but why is this necessary?  The other types haven't
> been lifted into constants...

Done.

[bartfab (slow), 2013-10-31 13:43:07]

lgtm

[Joao da Silva, 2013-11-07 12:46:29]

Committed patchset #6 manually as r233597 (presubmit successful).