Pepper: Move manifest logic to components/nacl.

Pepper: Move manifest logic to components/nacl.

The goal of the trusted plugin refactor is to move as much code as possible out of ppapi/native_client/src/trusted/plugin. This change moves all the manifest parsing and lookup logic out of the trusted plugin at once; it's hard to move piecemeal.

BUG=353592
R=bbudge@chromium.org, mallinath@chromium.org

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

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

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

[bbudge, 2014-05-01 23:32:33]

This is exciting.

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
File components/nacl/renderer/json_manifest.cc (left):

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/json_manifest.cc:23: #include
"third_party/jsoncpp/source/include/json/reader.h"
Nice to get rid of all this!

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
File components/nacl/renderer/json_manifest.h (right):

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/json_manifest.h:9: #include <string>
Blank line after this.

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/json_manifest.h:32: ErrorInfo* error_info);
Is this from clang format? Seems like it would fit on a single line.
Also the comment wording is awkward. How about this?

// Initializes the manifest object for use by later lookups.  Returns
// true if the manifest parses correctly and matches the schema.

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
File components/nacl/renderer/ppb_nacl_private_impl.cc (right):

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/ppb_nacl_private_impl.cc:90: const int kPNaClManifestId
= 0;
Could you make this the largest int32?

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/ppb_nacl_private_impl.cc:91: int g_next_manifest_id =
1;  // Hmm.
Then this could start at 0.

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/plugin.cc:375: uma_interface_(this) {
Need an initializer for manifest_id_.

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc (left):

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc:186: 
Is there a way to keep this output, perhaps from another place?

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/service_runtime.cc (right):

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/service_runtime.cc:265: &pp_mapped_url,
&pnacl_options)) {
Are you using clang-format? Some of the indenting in this CL seems inconsistent
- for example, that the first parameter is not on the line above.

[teravest, 2014-05-02 14:23:15]

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
File components/nacl/renderer/json_manifest.h (right):

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/json_manifest.h:9: #include <string>
On 2014/05/01 23:32:33, bbudge wrote:
> Blank line after this.

Done.

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/json_manifest.h:32: ErrorInfo* error_info);
Joined to one line, and changed to use your comment.
That was me formatting badly, not clang's fault. :)
Is there a way to run clang-format just over the change I've made instead of
worrying about touching the rest of the file(s)?

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
File components/nacl/renderer/ppb_nacl_private_impl.cc (right):

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/ppb_nacl_private_impl.cc:90: const int kPNaClManifestId
= 0;
On 2014/05/01 23:32:33, bbudge wrote:
> Could you make this the largest int32?

Done.

https://codereview.chromium.org/264943003/diff/70022/components/nacl/renderer...
components/nacl/renderer/ppb_nacl_private_impl.cc:91: int g_next_manifest_id =
1;  // Hmm.
On 2014/05/01 23:32:33, bbudge wrote:
> Then this could start at 0.

Done. That's better, thanks.

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/plugin.cc (right):

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/plugin.cc:375: uma_interface_(this) {
On 2014/05/01 23:32:33, bbudge wrote:
> Need an initializer for manifest_id_.

Done.

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc (left):

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc:186: 
I've restored this line and print the manifest_id_ now.

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/service_runtime.cc (right):

https://codereview.chromium.org/264943003/diff/70022/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/service_runtime.cc:265: &pp_mapped_url,
&pnacl_options)) {
On 2014/05/01 23:32:33, bbudge wrote:
> Are you using clang-format? Some of the indenting in this CL seems
inconsistent
> - for example, that the first parameter is not on the line above.

I've fixed up the formatting here.

[teravest, 2014-05-02 15:46:04]

+mallinath for adding third_party/jsoncpp/source/include/json to DEPS.

[Mallinath (Gone from Chromium), 2014-05-02 16:41:27]

lgtm for DEPS changes

[teravest, 2014-05-05 16:47:16]

Bill,

Mind taking another look?
I'm eager to get this one in since it touches a bunch of files.

Thanks!

[bbudge, 2014-05-05 16:49:43]

LGTM Thanks

[teravest, 2014-05-05 17:23:43]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-05-05 17:24:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/264943003/130001

[teravest, 2014-05-05 19:42:13]

Committed patchset #4 manually as r268250 (presubmit successful).

[teravest, 2014-05-05 21:28:12]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-05-05 21:28:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/264943003/150001

[teravest, 2014-05-05 21:46:45]

Committed patchset #5 manually as r268280 (presubmit successful).

[teravest, 2014-05-06 14:45:46]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-05-06 14:46:13]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/teravest@chromium.org/264943003/190001

[teravest, 2014-05-06 16:50:56]

Committed patchset #7 manually as r268535 (presubmit successful).