Stop adding the "files/" prefix when sending open_resource IPC

Stop adding the "files/" prefix when sending open_resource IPC

This is for simplifying https://codereview.chromium.org/1010183002/

This also removes 'if (key == kProgramKey) ...' check from
JsonManifest::ResolveKey. This is safe because kProgram key was
never passed to the function before the change.

BUG=nativeclient:3802
TEST=git cl try, manually checked that both ARC and PNaCl demo still work

Committed: https://crrev.com/7dd7f65010a485d31fb7660b0f47d36d4ac40a6c
Cr-Commit-Position: refs/heads/master@{#325547}

[Mark Seaborn, 2015-04-16 17:26:29]

mseaborn@chromium.org changed reviewers:
+ mseaborn@chromium.org

[Mark Seaborn, 2015-04-16 17:26:29]

Would it be possible to do this cleanup before
https://codereview.chromium.org/1010183002/ rather than after it?  That would be
a little easier to follow.

[Yusuke Sato, 2015-04-16 17:41:17]

On 2015/04/16 17:26:29, Mark Seaborn wrote:
> Would it be possible to do this cleanup before
> https://codereview.chromium.org/1010183002/ rather than after it?  That would
be
> a little easier to follow.

sure, doing.

[Yusuke Sato, 2015-04-16 18:26:26]

Mark, please take a look.

[Mark Seaborn, 2015-04-16 18:47:47]

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
File components/nacl/renderer/json_manifest.cc (right):

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:466: // key must be one of kProgramKey
or kFileKey '/' file-section-key
Update this comment (see my comment below)

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:470: if (key == kProgramKey)
Doesn't this case need to be removed now?

Otherwise, if you have a manifest file like this:

{
  "program": { "x86-32": ..., ... },
  "files": {
    "program": { "x86-32": ..., ... }
  }
}

then open_resource("program") will wrongly return the first entry instead of the
second.

AFAICT, before your change, no-one passes "program" to ResolveKey() (the code
uses GetProgramURL() instead), so it's safe to remove this case.

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:473: // generalize to permit other
sections?
You can remove this since comment it's clear we don't need to generalise this.

[Yusuke Sato, 2015-04-16 20:37:21]

Please take another look

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
File components/nacl/renderer/json_manifest.cc (right):

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:466: // key must be one of kProgramKey
or kFileKey '/' file-section-key
On 2015/04/16 18:47:47, Mark Seaborn wrote:
> Update this comment (see my comment below)

Done. I guess we can remove the comment.

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:470: if (key == kProgramKey)
On 2015/04/16 18:47:47, Mark Seaborn wrote:
> Doesn't this case need to be removed now?
> 
> Otherwise, if you have a manifest file like this:
> 
> {
>   "program": { "x86-32": ..., ... },
>   "files": {
>     "program": { "x86-32": ..., ... }
>   }
> }
> 
> then open_resource("program") will wrongly return the first entry instead of
the
> second.
> 
> AFAICT, before your change, no-one passes "program" to ResolveKey() (the code
> uses GetProgramURL() instead), so it's safe to remove this case.

Done.

https://codereview.chromium.org/1070233007/diff/20001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:473: // generalize to permit other
sections?
On 2015/04/16 18:47:47, Mark Seaborn wrote:
> You can remove this since comment it's clear we don't need to generalise this.

Done.

[Mark Seaborn, 2015-04-16 20:45:14]

"manualy" -> "manually" in commit message.  LGTM.

https://codereview.chromium.org/1070233007/diff/40001/components/nacl/rendere...
File components/nacl/renderer/json_manifest.cc (left):

https://codereview.chromium.org/1070233007/diff/40001/components/nacl/rendere...
components/nacl/renderer/json_manifest.cc:470: if (key == kProgramKey)
In the commit message, can you comment that kProgramKey was never passed here
before the change?

[Yusuke Sato, 2015-04-16 20:54:36]

On 2015/04/16 20:45:14, Mark Seaborn wrote:
> "manualy" -> "manually" in commit message.  LGTM.

oops, done

> 
>
https://codereview.chromium.org/1070233007/diff/40001/components/nacl/rendere...
> File components/nacl/renderer/json_manifest.cc (left):
> 
>
https://codereview.chromium.org/1070233007/diff/40001/components/nacl/rendere...
> components/nacl/renderer/json_manifest.cc:470: if (key == kProgramKey)
> In the commit message, can you comment that kProgramKey was never passed here
> before the change?

Done

[Yusuke Sato, 2015-04-16 20:54:57]

The CQ bit was checked by yusukes@chromium.org

[commit-bot: I haz the power, 2015-04-16 20:56:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070233007/40001

[commit-bot: I haz the power, 2015-04-16 21:10:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-16 21:10:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Yusuke Sato, 2015-04-16 21:21:20]

yusukes@chromium.org changed reviewers:
+ bbudge@chromium.org

[Yusuke Sato, 2015-04-16 21:21:35]

Bill, could you do an owner review for ppapi/?

[bbudge, 2015-04-16 21:23:07]

ppapi lgtm

[Yusuke Sato, 2015-04-16 21:28:24]

The CQ bit was checked by yusukes@chromium.org

[commit-bot: I haz the power, 2015-04-16 21:28:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070233007/40001

[commit-bot: I haz the power, 2015-04-16 22:08:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-16 22:08:49]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Yusuke Sato, 2015-04-16 22:10:35]

The CQ bit was checked by yusukes@chromium.org

[commit-bot: I haz the power, 2015-04-16 22:12:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070233007/40001

[commit-bot: I haz the power, 2015-04-16 23:27:07]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-16 23:28:24]

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