[NaCl SDK] Google Drive example

[NaCl SDK] Google Drive example

Also:
* generate app permissions automatically from examples
* stop using --incognito in make run
* use --user-data-dir instead, which is cleaned up in "make clean"
* share CHROME_ARGS with "make run_package" and "make run"

BUG=none
R=noelallen@chromium.org

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

[binji, 2013-04-29 18:48:24]

Take a look, this is the async version. I'm going to work on a blocking version
next.

https://codereview.chromium.org/14500010/diff/1/native_client_sdk/src/example...
File native_client_sdk/src/examples/demo/drive/example.js (right):

https://codereview.chromium.org/14500010/diff/1/native_client_sdk/src/example...
native_client_sdk/src/examples/demo/drive/example.js:62: /*
This is the JS equivalent code that is unused

[binji, 2013-04-30 00:45:15]

PTAL, much simpler due to using synchronous calls.

[noelallen1, 2013-04-30 18:46:08]

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
File native_client_sdk/src/examples/demo/drive/drive.cc (right):

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:53: void
AddQueryParameter(std::string* s,
Incorrect indent.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:314: Instance::~Instance() {}
required?

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:316: bool
Instance::Init(uint32_t argc, const char* argn[], const char* argv[]) {
Unused args.  We should be consistent, see file_io:
  virtual bool Init(uint32_t /*argc*/, const char* /*argn*/[],
      const char* /*argv*/[])

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:331: auth_token_ =
message.c_str() + strlen(kTokenMessage);
message.subst(strlen(kTokenMessage))?

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:331: auth_token_ =
message.c_str() + strlen(kTokenMessage);
Is this thread safe?  You are setting auth_token_ in one thread then using it in
the other?  Is it possible to get another auth_token message?

Would this all be cleaner if you used a message loop then let HandleMessage just
posted all string messages to the worker thread?  That should let you get rid of
the pthread mutex, cond_var, and simply issue a Run in your worker.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:333: message.c_str(),
kGetFileMessage, strlen(kGetFileMessage))) {
message.compare(0, strlen(kGetFileMessage), kGetFileMessage)?

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:384: static const char
kDescription[] = "A file generated by NaCl!";
Why is this local to ThreadRequest, but kToken is not local to HandleMessage?

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:425: PostMessage(output);
Why PostMessage here but not above?  Both cases are implicit pp::Var
constructors.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:432: p.q = "title = \'";
Var name p.q?

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
File native_client_sdk/src/examples/demo/drive/example.js (right):

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/example.js:41: common.hideModule();
We should probably make this the default behavior...

If a module does not set the size of the nacl embed, then start it big enough to
show the broken image, then hide it when loaded.

[binji, 2013-04-30 20:07:27]

Going to add some comments now.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
File native_client_sdk/src/examples/demo/drive/drive.cc (right):

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:53: void
AddQueryParameter(std::string* s,
On 2013/04/30 18:46:08, noelallen1 wrote:
> Incorrect indent.

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:314: Instance::~Instance() {}
On 2013/04/30 18:46:08, noelallen1 wrote:
> required?

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:316: bool
Instance::Init(uint32_t argc, const char* argn[], const char* argv[]) {
On 2013/04/30 18:46:08, noelallen1 wrote:
> Unused args.  We should be consistent, see file_io:
>   virtual bool Init(uint32_t /*argc*/, const char* /*argn*/[],
>       const char* /*argv*/[])
> 

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:331: auth_token_ =
message.c_str() + strlen(kTokenMessage);
On 2013/04/30 18:46:08, noelallen1 wrote:
> Is this thread safe?  You are setting auth_token_ in one thread then using it
in
> the other?  Is it possible to get another auth_token message?
> 
> Would this all be cleaner if you used a message loop then let HandleMessage
just
> posted all string messages to the worker thread?  That should let you get rid
of
> the pthread mutex, cond_var, and simply issue a Run in your worker.

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:333: message.c_str(),
kGetFileMessage, strlen(kGetFileMessage))) {
On 2013/04/30 18:46:08, noelallen1 wrote:
> message.compare(0, strlen(kGetFileMessage), kGetFileMessage)?

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:384: static const char
kDescription[] = "A file generated by NaCl!";
On 2013/04/30 18:46:08, noelallen1 wrote:
> Why is this local to ThreadRequest, but kToken is not local to HandleMessage?

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:425: PostMessage(output);
On 2013/04/30 18:46:08, noelallen1 wrote:
> Why PostMessage here but not above?  Both cases are implicit pp::Var
> constructors.

Done.

https://codereview.chromium.org/14500010/diff/4001/native_client_sdk/src/exam...
native_client_sdk/src/examples/demo/drive/drive.cc:432: p.q = "title = \'";
On 2013/04/30 18:46:08, noelallen1 wrote:
> Var name p.q?

Done.

[binji, 2013-05-02 23:11:02]

Added comments, modified app's manifest.json.

TODO: modify make RUN to run as packaged app.

[noelallen1, 2013-05-03 18:31:12]

LGTM

https://codereview.chromium.org/14500010/diff/13001/native_client_sdk/src/exa...
File native_client_sdk/src/examples/demo/drive/drive.cc (right):

https://codereview.chromium.org/14500010/diff/13001/native_client_sdk/src/exa...
native_client_sdk/src/examples/demo/drive/drive.cc:27: 
Comment, what is this?

https://codereview.chromium.org/14500010/diff/13001/native_client_sdk/src/exa...
native_client_sdk/src/examples/demo/drive/drive.cc:142: size_t num_bytes =
result > kReadBufferSize ? kReadBufferSize : result;
How can result be larger than kReadBufferSize?

[binji, 2013-05-03 23:00:39]

I'm going to wait until after the branch point to land.

https://codereview.chromium.org/14500010/diff/13001/native_client_sdk/src/exa...
File native_client_sdk/src/examples/demo/drive/drive.cc (right):

https://codereview.chromium.org/14500010/diff/13001/native_client_sdk/src/exa...
native_client_sdk/src/examples/demo/drive/drive.cc:27: 
On 2013/05/03 18:31:12, noelallen1 wrote:
> Comment, what is this?

Done.

https://codereview.chromium.org/14500010/diff/13001/native_client_sdk/src/exa...
native_client_sdk/src/examples/demo/drive/drive.cc:142: size_t num_bytes =
result > kReadBufferSize ? kReadBufferSize : result;
On 2013/05/03 18:31:12, noelallen1 wrote:
> How can result be larger than kReadBufferSize?

Done.

[binji, 2013-05-17 21:10:02]

PTAL:

* removed --incognito
* added --user-data-dir
* generate packaged app permissions from .dsc
* "make run_package" uses same CHROME_ARGS as "make run"

[noelallen1, 2013-05-21 22:58:50]

lgtm

[binji, 2013-05-22 00:40:28]

Committed patchset #8 manually as r201407 (presubmit successful).