Don't set lastError if chrome.dial.discoverNow() can't start.

Don't set lastError if chrome.dial.discoverNow() can't start; we always attempt discovery and success/failure is indicated through the callback.

Add browser test for chrome.dial.discoverNow() success.

BUG=376588
Committed: https://crrev.com/7bf382c276e3f5a29cae403991be5cb614fb3653
Cr-Commit-Position: refs/heads/master@{#292584}

[mark a. foltz, 2014-08-28 19:34:13]

mfoltz@chromium.org changed reviewers:
+ haibinlu@chromium.org, wez@chromium.org

[Wez, 2014-08-28 19:59:53]

LGTM but I think it would be helpful to clarify in the CL description why it is
that we don't want to set lastError in that case.

https://codereview.chromium.org/511943004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/dial/dial_apitest.cc (right):

https://codereview.chromium.org/511943004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/dial/dial_apitest.cc:84:
IN_PROC_BROWSER_TEST_F(DialAPITest, Discovery) {
So the Discovery test is now doing discovery _with_ listeners?

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/dial/experimental/discovery.js
(right):

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/dial/experimental/discovery.js:14:
chrome.test.assertTrue(result == true || result == false);
nit: All we're really asserting, then, is that typeof(result) == "boolean"?

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
File
chrome/test/data/extensions/api_test/dial/experimental/discovery_no_listeners.js
(right):

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/dial/experimental/discovery_no_listeners.js:10:
if (!result) {
nit: Should we still be asserting that typeof(result) == "boolean" before
testing that it equates to false?

[mark a. foltz, 2014-08-28 20:10:53]

On 2014/08/28 19:59:53, Wez wrote:
> LGTM but I think it would be helpful to clarify in the CL description why it
is
> that we don't want to set lastError in that case.

Done.

[mark a. foltz, 2014-08-28 20:14:50]

https://codereview.chromium.org/511943004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/dial/dial_apitest.cc (right):

https://codereview.chromium.org/511943004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/dial/dial_apitest.cc:84:
IN_PROC_BROWSER_TEST_F(DialAPITest, Discovery) {
On 2014/08/28 19:59:52, Wez wrote:
> So the Discovery test is now doing discovery _with_ listeners?

Acknowledged.

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
File chrome/test/data/extensions/api_test/dial/experimental/discovery.js
(right):

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/dial/experimental/discovery.js:14:
chrome.test.assertTrue(result == true || result == false);
On 2014/08/28 19:59:52, Wez wrote:
> nit: All we're really asserting, then, is that typeof(result) == "boolean"?

Yes, that's clearer.

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
File
chrome/test/data/extensions/api_test/dial/experimental/discovery_no_listeners.js
(right):

https://codereview.chromium.org/511943004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/dial/experimental/discovery_no_listeners.js:10:
if (!result) {
On 2014/08/28 19:59:52, Wez wrote:
> nit: Should we still be asserting that typeof(result) == "boolean" before
> testing that it equates to false?

Done.

[mark a. foltz, 2014-08-28 21:51:15]

The CQ bit was checked by mfoltz@chromium.org

[commit-bot: I haz the power, 2014-08-28 21:51:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mfoltz@chromium.org/511943004/20001

[commit-bot: I haz the power, 2014-08-28 23:15:52]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-28 23:25:52]

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

[commit-bot: I haz the power, 2014-08-28 23:25:53]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[mark a. foltz, 2014-08-29 06:38:56]

The CQ bit was checked by mfoltz@chromium.org

[commit-bot: I haz the power, 2014-08-29 06:39:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mfoltz@chromium.org/511943004/20001

[commit-bot: I haz the power, 2014-08-29 07:32:17]

Committed patchset #2 (id:20001) as e09b2f2d3787f44ea8bacceeafb2d9dcbae35791

[commit-bot: I haz the power, 2014-09-10 03:06:47]

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