cros: Add e2e tests for new_note action handler. Flip feature to stable.

cros: Add e2e tests for new_note action handler. Flip feature to stable.

BUG=657139
Review-Url: https://codereview.chromium.org/2693053002
Cr-Commit-Position: refs/heads/master@{#451860}
Committed: https://chromium.googlesource.com/chromium/src/+/0068bc112d1ffb74bb19a28641bab499766854d7

[jdufault, 2017-02-14 02:44:32]

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

[commit-bot: I haz the power, 2017-02-14 02:45:13]

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

[jdufault, 2017-02-14 02:46:46]

Description was changed from

==========
cros: Add e2e tests for new_note action handler. Flip to stable.

BUG=
==========

to

==========
cros: Add e2e tests for new_note action handler. Flip feature to stable.

BUG=657139
==========

[commit-bot: I haz the power, 2017-02-14 04:48:54]

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

[commit-bot: I haz the power, 2017-02-14 04:48:55]

Dry run: This issue passed the CQ dry run.

[jdufault, 2017-02-14 18:35:02]

jdufault@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[jdufault, 2017-02-14 18:36:06]

rdcronin@ PTAL. Thanks!

[Devlin, 2017-02-17 23:43:06]

lgtm, assuming that the CrOS side (the steps leading to LaunchAppWithAction) is
more thoroughly tested elsewhere?

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
File
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc
(right):

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc:17:
class ActionHandlersBrowserTest : public ExtensionApiTest {};
nit: prefer just "using ActionHandlersBrowserTest = ExtensionApiTest;"

the surrounding namespace is also unnecessary (though not harmful)

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc:27:
.AppendASCII("api_test")
test_data_dir_ should already point to chrome/test/data/extensions/api_test
so this could just be
base::FilePath path =
test_data_dir_.AppendASCII("action_handlers").AppendASCII("new_note");

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc:31:
CHECK(app);
s/CHECK/ASSERT_TRUE

https://codereview.chromium.org/2693053002/diff/1/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/action_handlers/new_note/manifest.json
(right):

https://codereview.chromium.org/2693053002/diff/1/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/action_handlers/new_note/manifest.json:5: 
nit: we don't really ever have empty lines in manifests.

[jdufault, 2017-02-21 19:02:56]

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

[commit-bot: I haz the power, 2017-02-21 19:04:14]

Dry run: 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, 2017-02-21 21:06:41]

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

[commit-bot: I haz the power, 2017-02-21 21:06:42]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[jdufault, 2017-02-21 21:07:41]

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
File
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc
(right):

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc:17:
class ActionHandlersBrowserTest : public ExtensionApiTest {};
On 2017/02/17 23:43:06, Devlin wrote:
> nit: prefer just "using ActionHandlersBrowserTest = ExtensionApiTest;"
> 
> the surrounding namespace is also unnecessary (though not harmful)

Done.

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc:27:
.AppendASCII("api_test")
On 2017/02/17 23:43:06, Devlin wrote:
> test_data_dir_ should already point to chrome/test/data/extensions/api_test
> so this could just be
> base::FilePath path =
> test_data_dir_.AppendASCII("action_handlers").AppendASCII("new_note");

Done.

https://codereview.chromium.org/2693053002/diff/1/chrome/browser/chromeos/ext...
chrome/browser/chromeos/extensions/action_handlers/action_handlers_apitest.cc:31:
CHECK(app);
On 2017/02/17 23:43:06, Devlin wrote:
> s/CHECK/ASSERT_TRUE

Done.

https://codereview.chromium.org/2693053002/diff/1/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/action_handlers/new_note/manifest.json
(right):

https://codereview.chromium.org/2693053002/diff/1/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/action_handlers/new_note/manifest.json:5: 
On 2017/02/17 23:43:06, Devlin wrote:
> nit: we don't really ever have empty lines in manifests.

Done.

[jdufault, 2017-02-21 21:07:51]

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

[jdufault, 2017-02-21 21:08:16]

derat@ PTAL at chrome/browser/chromeos/*

[Daniel Erat, 2017-02-21 21:28:28]

lgtm for c/b/chromeos

[jdufault, 2017-02-21 21:42:50]

The CQ bit was checked by jdufault@chromium.org

[jdufault, 2017-02-21 21:42:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2693053002/#ps40001
(title: "Address comments")

[commit-bot: I haz the power, 2017-02-21 21:44:10]

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, 2017-02-21 23:47:18]

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

[commit-bot: I haz the power, 2017-02-21 23:47:20]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[jdufault, 2017-02-21 23:49:10]

The CQ bit was checked by jdufault@chromium.org

[commit-bot: I haz the power, 2017-02-21 23:51:10]

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, 2017-02-22 01:00:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487720950473680,
"parent_rev": "68007db1623db39c01848788a21a9dde0031dbcb", "commit_rev":
"0068bc112d1ffb74bb19a28641bab499766854d7"}

[commit-bot: I haz the power, 2017-02-22 01:01:27]

Description was changed from

==========
cros: Add e2e tests for new_note action handler. Flip feature to stable.

BUG=657139
==========

to

==========
cros: Add e2e tests for new_note action handler. Flip feature to stable.

BUG=657139

Review-Url: https://codereview.chromium.org/2693053002
Cr-Commit-Position: refs/heads/master@{#451860}
Committed:
https://chromium.googlesource.com/chromium/src/+/0068bc112d1ffb74bb19a28641ba...
==========

[commit-bot: I haz the power, 2017-02-22 01:01:29]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/0068bc112d1ffb74bb19a28641ba...