Implementation of the device attributes API.

Implementation of the device attributes API.

chrome.enterprise.DeviceAttributes.getDirectoryDeviceId().
An API to allow extensions to get the cloud directory API device Id.

BUG=502330
Committed: https://crrev.com/97b25ca5565f00404e32f4b3215d207b3a79399b
Cr-Commit-Position: refs/heads/master@{#342331}

[Polina Bondarenko, 2015-07-20 14:14:48]

Patchset #1 (id:1) has been deleted

[Polina Bondarenko, 2015-07-20 14:22:21]

Patchset #1 (id:20001) has been deleted

[Polina Bondarenko, 2015-07-20 14:34:41]

pbond@chromium.org changed reviewers:
+ cschuet@chromium.org

[Polina Bondarenko, 2015-07-20 14:34:42]

Hi Christoph,
please take a look at this Extension API.

[Polina Bondarenko, 2015-07-22 14:38:04]

pbond@chromium.org changed reviewers:
+ kalman@chromium.org, pneubeck@chromium.org

[Polina Bondarenko, 2015-07-22 14:38:05]

Hi, there is implementation of the device attributes API.
The new Extension API proposal can be found here:
https://docs.google.com/document/d/1olXGYjOQb1VwsreOx3z2RR0POBHwkCOxyCGNj1lnU...

pneubeck@chromium.org: Please review changes in 
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.cc

kalman@chromium.org: Please review changes in 
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.h
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
chrome/chrome_browser_extensions.gypi
chrome/chrome_tests.gypi
chrome/common/extensions/api/_api_features.json
chrome/common/extensions/api/_permission_features.json
chrome/common/extensions/api/schemas.gypi
chrome/common/extensions/docs/templates/intros/enterprise_deviceAttributes.html
chrome/common/extensions/docs/templates/public/extensions/enterprise_deviceAttributes.html
chrome/common/extensions/permissions/chrome_api_permissions.cc	
extensions/browser/extension_function_histogram_value.h
extensions/common/permissions/api_permission.h
tools/metrics/histograms/histograms.xml

[not at google - send to devlin, 2015-07-22 16:21:09]

> kalman@chromium.org: Please review changes in 
>
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.h
>
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc
>
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc

Could you put an OWNERS in
chrome/browser/extensions/api/enterprise_device_attributes with yourself and at
least one other owner who can do reviews for you?

[not at google - send to devlin, 2015-07-22 16:22:53]

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
chrome/common/extensions/api/_permission_features.json:349:
"component_extensions_auto_granted": false,
Why do you need this?

(I mean, every API should have this, but why this one in particular?)

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
File
chrome/common/extensions/docs/templates/public/extensions/enterprise_deviceAttributes.html
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
chrome/common/extensions/docs/templates/public/extensions/enterprise_deviceAttributes.html:1:
{{+partials.standard_extensions_api
api:apis.extensions.enterprise_deviceAttributes
intro:intros.enterprise_deviceAttributes chrome_os_only:true/}}
Is this what the other policy-only APIs do? We should really generate this
rather than needing people to write the templates manually. Maybe you can file a
bug about that.

[pneubeck (no reviews), 2015-07-23 12:53:22]

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc:51:
std::string directoryDeviceId = GetDirectoryDeviceId(browser_context());
s/directoryDeviceId/directory_device_id/
s/std::string/const std::string/

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc:52:
scoped_ptr<base::Value> result(new base::StringValue(directoryDeviceId));
please use the generated result wrappers.
they should be in 
out/Release/gen/chrome/common/extensions/api/enterprise_device_attributes.h

that you can include by
#include "chrome/common/extensions/api/enterprise_device_attributes.h"

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc:54:
SendResponse(true);
once you changed to the other base class, you'll return the result using
  return RespondNow(...);

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.h
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.h:13:
: public AsyncExtensionFunction {
i think you shouldn't inherit from that class anymore.
see
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

instead derive directly from UIThreadExtensionFunction

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:17:
const std::string kDeviceId = "device_id";
nit: indentation is wrong (just run 'git cl format')

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:22:
// Base class for apitests - inits the testing enviroment init, provides API for
this comment can be removed

instead rename it to include 'Test' in the class name.
you can also move it into the anonymous namespace

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:28:

nit: remove empty line

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:71:
// The get_device_id_function - is a permission feature.
I don't understand this part of the comment

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:76:
get_device_id_function->set_name(feature_full_name.c_str());
as far as I understand, you're testing several orthogonal things here:

1) Whether the getDirectoryDeviceId function is returning the right result
depending on the user affiliation

2) Whether ...function->HasPermission() is returning the right result

Only for 2) you seem to have to call set_name with the right argument.
I think you should either rely on 2) to be implemented correctly in the
extension system or at least separate that into another test.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:114:
// Creates affiliated user before browser init.
nit: s/init/initializes/

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:124:
const std::string kDomain = "gmail.com";
could you instead pass this to the Base constructor or move it to a virtual
function that you override in each derived class?
That way you could remove SetUpInProcess... from the derived classes.
And move InitTest into
EnterpriseDeviceAttributesBase::SetUpInProcessBrowserTestFixture

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:127:
// Creates non-affiliated user before browser init.
nit: s/init/initializes/

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:140:
// Tests the case when the user is affiliated and the extension has been
better describe what the rule or fact is that you want to check.
currently you're only describing the precondition and don't mention the
expectation.

same for the other tests.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:149:
RunTest(kDeviceId, Manifest::EXTERNAL_COMPONENT);
why component?
you might instead want to ensure that user downloaded extensions are not able to
use the api:  EXTERNAL_PREF_DOWNLOAD

the expected result of the function should then be "" and not kDeviceId.

[Polina Bondarenko, 2015-07-24 16:44:23]

Patchset #2 (id:60001) has been deleted

[Polina Bondarenko, 2015-07-24 16:47:59]

Fixed comments, added a couple browser tests.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc:51:
std::string directoryDeviceId = GetDirectoryDeviceId(browser_context());
On 2015/07/23 12:53:21, pneubeck wrote:
> s/directoryDeviceId/directory_device_id/
> s/std::string/const std::string/

Done, renamed to device_id.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc:52:
scoped_ptr<base::Value> result(new base::StringValue(directoryDeviceId));
On 2015/07/23 12:53:21, pneubeck wrote:
> please use the generated result wrappers.
> they should be in 
> out/Release/gen/chrome/common/extensions/api/enterprise_device_attributes.h
> 
> that you can include by
> #include "chrome/common/extensions/api/enterprise_device_attributes.h"

Done, but the ListValue is returned now, is it ok?

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc:54:
SendResponse(true);
On 2015/07/23 12:53:21, pneubeck wrote:
> once you changed to the other base class, you'll return the result using
>   return RespondNow(...);

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.h
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.h:13:
: public AsyncExtensionFunction {
On 2015/07/23 12:53:22, pneubeck wrote:
> i think you shouldn't inherit from that class anymore.
> see
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> 
> instead derive directly from UIThreadExtensionFunction

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:17:
const std::string kDeviceId = "device_id";
On 2015/07/23 12:53:22, pneubeck wrote:
> nit: indentation is wrong (just run 'git cl format')

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:22:
// Base class for apitests - inits the testing enviroment init, provides API for
On 2015/07/23 12:53:22, pneubeck wrote:
> this comment can be removed
> 
> instead rename it to include 'Test' in the class name.
> you can also move it into the anonymous namespace

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:28:

On 2015/07/23 12:53:22, pneubeck wrote:
> nit: remove empty line

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:71:
// The get_device_id_function - is a permission feature.
On 2015/07/23 12:53:22, pneubeck wrote:
> I don't understand this part of the comment

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:76:
get_device_id_function->set_name(feature_full_name.c_str());
On 2015/07/23 12:53:22, pneubeck wrote:
> as far as I understand, you're testing several orthogonal things here:
> 
> 1) Whether the getDirectoryDeviceId function is returning the right result
> depending on the user affiliation
> 
> 2) Whether ...function->HasPermission() is returning the right result
> 
> Only for 2) you seem to have to call set_name with the right argument.
> I think you should either rely on 2) to be implemented correctly in the
> extension system or at least separate that into another test.

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:114:
// Creates affiliated user before browser init.
On 2015/07/23 12:53:22, pneubeck wrote:
> nit: s/init/initializes/

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:124:
const std::string kDomain = "gmail.com";
On 2015/07/23 12:53:22, pneubeck wrote:
> could you instead pass this to the Base constructor or move it to a virtual
> function that you override in each derived class?
> That way you could remove SetUpInProcess... from the derived classes.
> And move InitTest into
> EnterpriseDeviceAttributesBase::SetUpInProcessBrowserTestFixture

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:127:
// Creates non-affiliated user before browser init.
On 2015/07/23 12:53:22, pneubeck wrote:
> nit: s/init/initializes/

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:140:
// Tests the case when the user is affiliated and the extension has been
On 2015/07/23 12:53:22, pneubeck wrote:
> better describe what the rule or fact is that you want to check.
> currently you're only describing the precondition and don't mention the
> expectation.
> 
> same for the other tests.

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:149:
RunTest(kDeviceId, Manifest::EXTERNAL_COMPONENT);
On 2015/07/23 12:53:22, pneubeck wrote:
> why component?
> you might instead want to ensure that user downloaded extensions are not able
to
> use the api:  EXTERNAL_PREF_DOWNLOAD
> 
> the expected result of the function should then be "" and not kDeviceId.

Done.

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
chrome/common/extensions/api/_permission_features.json:349:
"component_extensions_auto_granted": false,
On 2015/07/22 16:22:53, kalman wrote:
> Why do you need this?
> 
> (I mean, every API should have this, but why this one in particular?)

Removed.

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
File
chrome/common/extensions/docs/templates/public/extensions/enterprise_deviceAttributes.html
(right):

https://codereview.chromium.org/1240283002/diff/40001/chrome/common/extension...
chrome/common/extensions/docs/templates/public/extensions/enterprise_deviceAttributes.html:1:
{{+partials.standard_extensions_api
api:apis.extensions.enterprise_deviceAttributes
intro:intros.enterprise_deviceAttributes chrome_os_only:true/}}
On 2015/07/22 16:22:53, kalman wrote:
> Is this what the other policy-only APIs do? We should really generate this
> rather than needing people to write the templates manually. Maybe you can file
a
> bug about that.

Yes, it is. I've just copied this line from another.

[Polina Bondarenko, 2015-07-29 09:47:56]

pbond@chromium.org changed reviewers:
+ isherman@chromium.org

[Polina Bondarenko, 2015-07-29 09:47:56]

isherman@chromium.org: Please review changes in 

extensions/browser/extension_function_histogram_value.h

tools/metrics/histograms/histograms.xml

[Ilya Sherman, 2015-08-03 22:25:15]

histograms lgtm

[pneubeck (no reviews), 2015-08-05 12:19:20]

lgtm

only a few minor nits

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h (right):

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h:71: //
Returns the cloud directory API ID.
for completeness, you could mention that it returns the empty string if thhe ID
is not set.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
(right):

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:13:
#include "extensions/common/manifest_constants.h"
this and the includes below may not be required anymore

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:22:
class EnterpriseDeviceAttributesTest : public ExtensionApiTest {
nit: empty line before this one

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:33:
scoped_ptr<chromeos::SessionManagerClient>(
does make_scoped_ptr work here? (you would not have to specify template
arguments)

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:39:
// Inits the device policy and install attributes, should be invoked before
you could inline this function into SetUpInProcessBrowserTestFixture
i think this separation doesn't provide any additional clarity.

then you don't have to comment when to invoke this function.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:74:
chromeos::FakeSessionManagerClient* fake_session_manager_client_;
nit: const

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:76:
std::string test_domain_;
nit: const

[Polina Bondarenko, 2015-08-05 14:15:12]

Fixed the last pneubeck@'s comments.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/chromeo...
File chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h (right):

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/chromeo...
chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h:71: //
Returns the cloud directory API ID.
On 2015/08/05 12:19:20, pneubeck wrote:
> for completeness, you could mention that it returns the empty string if thhe
ID
> is not set.

Done.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
File
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
(right):

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:13:
#include "extensions/common/manifest_constants.h"
On 2015/08/05 12:19:20, pneubeck wrote:
> this and the includes below may not be required anymore

Done.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:22:
class EnterpriseDeviceAttributesTest : public ExtensionApiTest {
On 2015/08/05 12:19:20, pneubeck wrote:
> nit: empty line before this one

Done.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:33:
scoped_ptr<chromeos::SessionManagerClient>(
On 2015/08/05 12:19:20, pneubeck wrote:
> does make_scoped_ptr work here? (you would not have to specify template
> arguments)

Sure, done.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:39:
// Inits the device policy and install attributes, should be invoked before
On 2015/08/05 12:19:20, pneubeck wrote:
> you could inline this function into SetUpInProcessBrowserTestFixture
> i think this separation doesn't provide any additional clarity.
> 
> then you don't have to comment when to invoke this function.

Done.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:74:
chromeos::FakeSessionManagerClient* fake_session_manager_client_;
On 2015/08/05 12:19:20, pneubeck wrote:
> nit: const

Some non-const methods are invoked from this instance, that's why it is not
const here, but sure, I made test_domain_ and get_device_id_function_ const.

https://codereview.chromium.org/1240283002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc:76:
std::string test_domain_;
On 2015/08/05 12:19:20, pneubeck wrote:
> nit: const

Done.

[Polina Bondarenko, 2015-08-05 15:45:07]

Patchset #5 (id:140001) has been deleted

[not at google - send to devlin, 2015-08-05 15:52:24]

lgtm

https://codereview.chromium.org/1240283002/diff/160001/chrome/common/extensio...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/1240283002/diff/160001/chrome/common/extensio...
chrome/common/extensions/api/_permission_features.json:352: "extension_types":
["extension", "legacy_packaged_app", "platform_app"],
Let's remove legacy_packaged_app. Nobody should be writing legacy packaged apps.
Pity they made it into the other enterprise.* APIs, we can remove those in a
separate patch.

[not at google - send to devlin, 2015-08-05 15:52:52]

btw please file that bug I mentioned with generating the policy documentation.

[Polina Bondarenko, 2015-08-06 08:30:53]

Patchset #7 (id:200001) has been deleted

[Polina Bondarenko, 2015-08-06 11:24:55]

pbond@chromium.org changed reviewers:
+ thakis@chromium.org

[Polina Bondarenko, 2015-08-06 11:24:55]

thakis@chromium.org: Please review changes in 

chrome/test/BUILD.gn

This extension API works only on Chrome OS, that's why I remove test file from
non-chromeos source files.

https://codereview.chromium.org/1240283002/diff/160001/chrome/common/extensio...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/1240283002/diff/160001/chrome/common/extensio...
chrome/common/extensions/api/_permission_features.json:352: "extension_types":
["extension", "legacy_packaged_app", "platform_app"],
On 2015/08/05 15:52:24, kalman wrote:
> Let's remove legacy_packaged_app. Nobody should be writing legacy packaged
apps.
> Pity they made it into the other enterprise.* APIs, we can remove those in a
> separate patch.

Done.

[Polina Bondarenko, 2015-08-07 08:32:03]

pbond@chromium.org changed reviewers:
- thakis@chromium.org

[Polina Bondarenko, 2015-08-07 08:34:22]

pbond@chromium.org changed reviewers:
+ jochen@chromium.org

[Polina Bondarenko, 2015-08-07 08:34:23]

jochen@chromium.org: Please review changes in 

chrome/test/BUILD.gn

This extension API works only on Chrome OS, that's why I remove test file from
non-chromeos source files.

[jochen (gone - plz use gerrit), 2015-08-07 08:36:45]

lgtm

[Polina Bondarenko, 2015-08-07 08:46:07]

The CQ bit was checked by pbond@chromium.org

[Polina Bondarenko, 2015-08-07 08:46:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, pneubeck@chromium.org, kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1240283002/#ps220001
(title: "Fixed build.")

[commit-bot: I haz the power, 2015-08-07 08:46:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1240283002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1240283002/220001

[Polina Bondarenko, 2015-08-07 08:46:50]

Thanks!

[commit-bot: I haz the power, 2015-08-07 10:59:36]

Committed patchset #7 (id:220001)

[commit-bot: I haz the power, 2015-08-07 11:00:08]

Patchset 7 (id:??) landed as
https://crrev.com/97b25ca5565f00404e32f4b3215d207b3a79399b
Cr-Commit-Position: refs/heads/master@{#342331}