Updates permissions and feedback whilelist.

Updates permissions and feedback whilelist.

* Removes the recovery tool from the feedbackPrivate API
* Adds a new ID for a dev version of the tool to chromeosInfoPrivate and imageWriterPrivate
* Adds both IDs to the feedback extension's whitelist.

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

[Drew Haven, 2013-12-19 20:08:02]

As I understood your comments we should be whitelisting in the feedback
resources and not in the permissions.  Can you take a look at this CL to
approve?  If you don't have owners on the events.js file, do you know a good
person to approve?  Perhaps Xiyuan?

I can find someone for the _permissions_features.json file.

[Drew Haven, 2013-12-19 20:08:55]

+CC nickam, stephenlin

[stephenlin, 2013-12-19 20:32:50]

https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
File chrome/browser/resources/feedback/js/event_handler.js (right):

https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
chrome/browser/resources/feedback/js/event_handler.js:28:
'jndclpdbaamdhonoechobihbbiimdgai'  // Chrome OS Recovery Tool
Think we should create our dev/staging IDs and add them now so we don't have to
deal with this later.
No obfuscation of the ID?

[rkc, 2013-12-19 21:21:13]

You don't need to add the ID to the _permission_features.json (that is only for
apps that will be using the Feedback API directly - and we only want the one app
listed there to use it). Just adding it to the event_handler.js should be fine,
and yes, xiyuan@chromium.org can LGTM it.

[Drew Haven, 2013-12-20 01:00:13]

I added the new ID.

Xiyuan, could you take a look at this LGTM the changes for the feedback tool
whitelist?  I think I'll still need another LGTM for the
_permissions_features.json

https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
File chrome/browser/resources/feedback/js/event_handler.js (right):

https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
chrome/browser/resources/feedback/js/event_handler.js:28:
'jndclpdbaamdhonoechobihbbiimdgai'  // Chrome OS Recovery Tool
On 2013/12/19 20:32:50, stephenlin wrote:
> Think we should create our dev/staging IDs and add them now so we don't have
to
> deal with this later.
> No obfuscation of the ID?

Apparently no obfuscation here.  I'll create another app and add one.

[rkc, 2013-12-20 03:46:26]

additions for the feedbackPrivate API to the _permission_features.json should
ONLY be for apps approved to use the feedbackAPI directly. This ID needs to be
removed from there before this CL can be submitted.

NOT LGTM till the id is removed from _permision_features.json

On 2013/12/20 01:00:13, Drew Haven wrote:
> I added the new ID.
> 
> Xiyuan, could you take a look at this LGTM the changes for the feedback tool
> whitelist?  I think I'll still need another LGTM for the
> _permissions_features.json
> 
>
https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
> File chrome/browser/resources/feedback/js/event_handler.js (right):
> 
>
https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
> chrome/browser/resources/feedback/js/event_handler.js:28:
> 'jndclpdbaamdhonoechobihbbiimdgai'  // Chrome OS Recovery Tool
> On 2013/12/19 20:32:50, stephenlin wrote:
> > Think we should create our dev/staging IDs and add them now so we don't have
> to
> > deal with this later.
> > No obfuscation of the ID?
> 
> Apparently no obfuscation here.  I'll create another app and add one.

[Drew Haven, 2013-12-20 07:54:25]

On 2013/12/20 03:46:26, Rahul Chaturvedi wrote:
> additions for the feedbackPrivate API to the _permission_features.json should
> ONLY be for apps approved to use the feedbackAPI directly. This ID needs to be
> removed from there before this CL can be submitted.
> 
> NOT LGTM till the id is removed from _permision_features.json
> 
> On 2013/12/20 01:00:13, Drew Haven wrote:
> > I added the new ID.
> > 
> > Xiyuan, could you take a look at this LGTM the changes for the feedback tool
> > whitelist?  I think I'll still need another LGTM for the
> > _permissions_features.json
> > 
> >
>
https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
> > File chrome/browser/resources/feedback/js/event_handler.js (right):
> > 
> >
>
https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
> > chrome/browser/resources/feedback/js/event_handler.js:28:
> > 'jndclpdbaamdhonoechobihbbiimdgai'  // Chrome OS Recovery Tool
> > On 2013/12/19 20:32:50, stephenlin wrote:
> > > Think we should create our dev/staging IDs and add them now so we don't
have
> > to
> > > deal with this later.
> > > No obfuscation of the ID?
> > 
> > Apparently no obfuscation here.  I'll create another app and add one.

Oh, sorry, that's what I get for trying to do two things at once.  This does not
add any IDs to that, and actually removes them.   This CL fixes the whitelisting
for the feedback for our app and adds a new ID for a dev app at the same time. 
That means we have the following changes:

Chrome OS Recovery Tool Prod
* Removed from feedbackPrivite
* Added to whitelist in event_handler.js

Chrome OS Recovery Tol Dev
* Added to whitelist in event_handler.js
* Added to whitelist for imageWriterPrivate
* Added to whitelist for chromeosInfoPrivate

Sorry about the confusion.

[rkc, 2013-12-20 10:03:43]

Ah I see.
LGTM then.

You will need owners approval for c/b/r and c/c/ext
I would suggest xiyuan@ and asargent@

On 2013/12/20 07:54:25, Drew Haven wrote:
> On 2013/12/20 03:46:26, Rahul Chaturvedi wrote:
> > additions for the feedbackPrivate API to the _permission_features.json
should
> > ONLY be for apps approved to use the feedbackAPI directly. This ID needs to
be
> > removed from there before this CL can be submitted.
> > 
> > NOT LGTM till the id is removed from _permision_features.json
> > 
> > On 2013/12/20 01:00:13, Drew Haven wrote:
> > > I added the new ID.
> > > 
> > > Xiyuan, could you take a look at this LGTM the changes for the feedback
tool
> > > whitelist?  I think I'll still need another LGTM for the
> > > _permissions_features.json
> > > 
> > >
> >
>
https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
> > > File chrome/browser/resources/feedback/js/event_handler.js (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/99463006/diff/1/chrome/browser/resources/feed...
> > > chrome/browser/resources/feedback/js/event_handler.js:28:
> > > 'jndclpdbaamdhonoechobihbbiimdgai'  // Chrome OS Recovery Tool
> > > On 2013/12/19 20:32:50, stephenlin wrote:
> > > > Think we should create our dev/staging IDs and add them now so we don't
> have
> > > to
> > > > deal with this later.
> > > > No obfuscation of the ID?
> > > 
> > > Apparently no obfuscation here.  I'll create another app and add one.
> 
> Oh, sorry, that's what I get for trying to do two things at once.  This does
not
> add any IDs to that, and actually removes them.   This CL fixes the
whitelisting
> for the feedback for our app and adds a new ID for a dev app at the same time.

> That means we have the following changes:
> 
> Chrome OS Recovery Tool Prod
> * Removed from feedbackPrivite
> * Added to whitelist in event_handler.js
> 
> Chrome OS Recovery Tol Dev
> * Added to whitelist in event_handler.js
> * Added to whitelist for imageWriterPrivate
> * Added to whitelist for chromeosInfoPrivate
> 
> Sorry about the confusion.

[xiyuan, 2013-12-20 16:07:03]

Stampy LGTM

[Drew Haven, 2013-12-20 17:41:07]

Antony,

In the last updates to the whitelist it turns out we shouldn't be using
feedbackPrivate directly, instead we have to whitelist with the feedback
extension.  Would you mind taking a look at this CL and approving the changes to
_permission_features.json?

Thanks.

-- Drew

[asargent_no_longer_on_chrome, 2013-12-21 00:13:50]

lgtm

[commit-bot: I haz the power, 2013-12-21 00:22:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haven@chromium.org/99463006/20001

[commit-bot: I haz the power, 2013-12-21 00:22:32]

Failed to apply patch for
chrome/common/extensions/api/_permission_features.json:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/common/extensions/api/_permission_features.json
  Hunk #1 FAILED at 181.
  Hunk #2 FAILED at 368.
  Hunk #3 succeeded at 701 (offset 3 lines).
  2 out of 3 hunks FAILED -- saving rejects to file
chrome/common/extensions/api/_permission_features.json.rej

Patch:       chrome/common/extensions/api/_permission_features.json
Index: chrome/common/extensions/api/_permission_features.json
diff --git a/chrome/common/extensions/api/_permission_features.json
b/chrome/common/extensions/api/_permission_features.json
index
ca14ceee217c48cf75174b189baca162257824b9..5133a51f7b7ddf9fa954e4d0e9d3ee5c8f5a13ef
100644
--- a/chrome/common/extensions/api/_permission_features.json
+++ b/chrome/common/extensions/api/_permission_features.json
@@ -181,6 +181,7 @@
       "A3BC37E2148AC4E99BE4B16AF9D42DD1E592BBBE",  // http://crbug.com/293683
       "8C3741E3AF0B93B6E8E0DDD499BB0B74839EA578",  // http://crbug.com/234235
       "E703483CEF33DEC18B4B6DD84B5C776FB9182BDB",  // http://crbug.com/234235
+      "314B0FC37CAE989AF42887E5303BD1E98BF12DDE",  // http://crbug.com/329088
       "D7986543275120831B39EF28D1327552FC343960"   // http://crbug.com/329088
     ]
   },
@@ -368,8 +369,7 @@
       "1C93BD3CF875F4A73C0B2A163BB8FBDA8B8B3D80",  // http://crbug.com/293683
       "A3BC37E2148AC4E99BE4B16AF9D42DD1E592BBBE",  // http://crbug.com/293683
       "8C3741E3AF0B93B6E8E0DDD499BB0B74839EA578",  // http://crbug.com/234235
-      "E703483CEF33DEC18B4B6DD84B5C776FB9182BDB",  // http://crbug.com/234235
-      "D7986543275120831B39EF28D1327552FC343960"   // http://crbug.com/329088
+      "E703483CEF33DEC18B4B6DD84B5C776FB9182BDB"   // http://crbug.com/234235
     ]
   },
   "fileBrowserHandler": {
@@ -698,6 +698,7 @@
     "channel": "stable",
     "extension_types": ["platform_app"],
     "whitelist": [
+      "314B0FC37CAE989AF42887E5303BD1E98BF12DDE", // Chrome OS Recovery Tool
Dev
       "D7986543275120831B39EF28D1327552FC343960"  // Chrome OS Recovery Tool
     ]
   },

[commit-bot: I haz the power, 2013-12-21 00:46:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haven@chromium.org/99463006/90001

[commit-bot: I haz the power, 2013-12-21 03:20:00]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-12-24 18:21:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haven@chromium.org/99463006/90001

[commit-bot: I haz the power, 2013-12-24 19:06:51]

Change committed as 242451