chromeos: Add note-taking-app utility functions.

chromeos: Add note-taking-app utility functions.

Add utility functions that can be used to check whether a
note-taking app is available and to launch it.

BUG=605805, 606398, 616112
Committed: https://crrev.com/d7ce54d935656d46f85fcfe201bf84239fa5ce50
Cr-Commit-Position: refs/heads/master@{#410450}

[Daniel Erat, 2016-08-05 22:36:59]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[Daniel Erat, 2016-08-05 22:37:13]

derat@chromium.org changed reviewers:
+ jdufault@chromium.org

[commit-bot: I haz the power, 2016-08-05 22:37:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jdufault, 2016-08-05 22:59:38]

https://codereview.chromium.org/2225543002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/note_taking_app_utils.cc (right):

https://codereview.chromium.org/2225543002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:75: // TODO: Launch with an
"action" launch source and the appropriate action type
FYI, I have a CL that I'll upload shortly that does this.

https://codereview.chromium.org/2225543002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:80:
apps::LaunchPlatformAppWithPath(profile, app, path);
Did you have a chance to take a look at the API proposal (link below)?

I think we should add a new action type (annotate_screenshot) with a LaunchItem
or just a file path associated with it inside of the ActionData.

https://docs.google.com/document/u/1/d/1f3zpf8RMxavy3oTYHjiWKcU-Luo9EV25RyIpn...

[Daniel Erat, 2016-08-05 23:19:23]

https://codereview.chromium.org/2225543002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/note_taking_app_utils.cc (right):

https://codereview.chromium.org/2225543002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:75: // TODO: Launch with an
"action" launch source and the appropriate action type
On 2016/08/05 22:59:38, jdufault wrote:
> FYI, I have a CL that I'll upload shortly that does this.

i'd read the doc and figured you were were working on the api change (hence the
mention of the "action" launch source). :-)

do you mean you have a changelist that does the same thing as this one, too?

https://codereview.chromium.org/2225543002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:80:
apps::LaunchPlatformAppWithPath(profile, app, path);
On 2016/08/05 22:59:38, jdufault wrote:
> Did you have a chance to take a look at the API proposal (link below)?
> 
> I think we should add a new action type (annotate_screenshot) with a
LaunchItem
> or just a file path associated with it inside of the ActionData.
> 
>
https://docs.google.com/document/u/1/d/1f3zpf8RMxavy3oTYHjiWKcU-Luo9EV25RyIpn...

i chatted with tom about this yesterday. i didn't see a compelling reason for an
"annotate_screenshot" action, as the behavior we want for v1 is actually "start
a new note with this image in it" rather than e.g. "open this image for
editing".

the app launcher code for passing file paths seems pretty complicated. i'd
suggest not reimplementing it and instead using the existing file handler code
unless there's a strong reason not to.

(happy to chat over vc or im if that'd help!)

[commit-bot: I haz the power, 2016-08-05 23:23:21]

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

[commit-bot: I haz the power, 2016-08-05 23:23:22]

Dry run: This issue passed the CQ dry run.

[Daniel Erat, 2016-08-06 00:22:23]

let me know if this looks okay. we'll definitely need to change it later to pass
an action; i just want to start iterating and land my UI change that uses this.
:-)

[Daniel Erat, 2016-08-08 18:47:12]

uh, where'd my changes go? :-/ let me try to figure out what went wrong here.

[Daniel Erat, 2016-08-08 19:25:39]

okay, mind taking a look at this?

[Daniel Erat, 2016-08-08 19:30:01]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 19:30:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jdufault, 2016-08-08 19:44:24]

lgtm

https://codereview.chromium.org/2225543002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/note_taking_app_utils.cc (right):

https://codereview.chromium.org/2225543002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:39: const std::string
switch_value =
Pull into a helper method? I'm fine as is.

https://codereview.chromium.org/2225543002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:81:
apps::LaunchPlatformApp(profile, app, extensions::SOURCE_UNTRACKED);
We should probably specify the launch source - add a TODO? I'm going to push to
remove the "action" launch source from the extension proposal.

[Daniel Erat, 2016-08-08 20:04:37]

https://codereview.chromium.org/2225543002/diff/60001/chrome/browser/chromeos...
File chrome/browser/chromeos/note_taking_app_utils.cc (right):

https://codereview.chromium.org/2225543002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:39: const std::string
switch_value =
On 2016/08/08 19:44:23, jdufault wrote:
> Pull into a helper method? I'm fine as is.

thanks, i'll probably leave it like this (since it's just a few lines, only used
in one spot, and temporary until we have a pref). :-)

https://codereview.chromium.org/2225543002/diff/60001/chrome/browser/chromeos...
chrome/browser/chromeos/note_taking_app_utils.cc:81:
apps::LaunchPlatformApp(profile, app, extensions::SOURCE_UNTRACKED);
On 2016/08/08 19:44:23, jdufault wrote:
> We should probably specify the launch source - add a TODO? I'm going to push
to
> remove the "action" launch source from the extension proposal.

do you think we should add new launch sources named e.g. "palette", "download",
and "screenshot" to used here? many of the functions in apps/launcher.h
(particularly the ones that pass files) don't take a launch source; i'll see
what gets passed when those are called (i wouldn't be surprised if it's
"file_handler").

i've added a TODO for now.

[Daniel Erat, 2016-08-08 20:08:39]

The CQ bit was checked by derat@chromium.org

[Daniel Erat, 2016-08-08 20:08:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
jdufault@chromium.org
Link to the patchset: https://codereview.chromium.org/2225543002/#ps80001
(title: "update TODO for review comment")

[commit-bot: I haz the power, 2016-08-08 20:09:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 20:44:59]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-08 20:47:16]

Description was changed from

==========
chromeos: Add note-taking-app utility functions.

Add utility functions that can be used to check whether a
note-taking app is available and to launch it.

BUG=605805,606398,616112
==========

to

==========
chromeos: Add note-taking-app utility functions.

Add utility functions that can be used to check whether a
note-taking app is available and to launch it.

BUG=605805,606398,616112

Committed: https://crrev.com/d7ce54d935656d46f85fcfe201bf84239fa5ce50
Cr-Commit-Position: refs/heads/master@{#410450}
==========

[commit-bot: I haz the power, 2016-08-08 20:47:18]

Patchset 5 (id:??) landed as
https://crrev.com/d7ce54d935656d46f85fcfe201bf84239fa5ce50
Cr-Commit-Position: refs/heads/master@{#410450}