Deliver errors from devtools API calls to callbacks

If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082
Committed: https://crrev.com/0c99aca28e9e59194c153ea13b85c1d7763467fb
Cr-Commit-Position: refs/heads/master@{#435240}

[tmarek, 2016-11-29 16:54:45]

Description was changed from

==========
If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082
==========

to

==========
If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082
==========

[tmarek, 2016-11-29 16:54:45]

tmarek@google.com changed reviewers:
+ altimin@chromium.org

[altimin, 2016-11-29 17:43:28]

Thanks for doing this! Couple of nits below.

Also, can you please add a header to commit description? It should be one-liner
with area identifier, something like "headless: Handle errors from DevTools"

https://codereview.chromium.org/2539833002/diff/1/headless/lib/browser/headle...
File headless/lib/browser/headless_devtools_client_impl.cc (right):

https://codereview.chromium.org/2539833002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_devtools_client_impl.cc:133:
callback.callback_with_result.Run(*base::Value::CreateNullValue());
I don't really like creating a temporary unique pointer and passing a const ref
to a function, it scares me. Can we please have an explicit variable
declaration?

https://codereview.chromium.org/2539833002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_devtools_client_impl.cc:135: NOTREACHED() <<
"Replay has neither result nor error";
nit: reply?

[tmarek, 2016-11-30 09:01:09]

https://codereview.chromium.org/2539833002/diff/1/headless/lib/browser/headle...
File headless/lib/browser/headless_devtools_client_impl.cc (right):

https://codereview.chromium.org/2539833002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_devtools_client_impl.cc:133:
callback.callback_with_result.Run(*base::Value::CreateNullValue());
On 2016/11/29 17:43:27, altimin wrote:
> I don't really like creating a temporary unique pointer and passing a const
ref
> to a function, it scares me. Can we please have an explicit variable
> declaration?

Done.

https://codereview.chromium.org/2539833002/diff/1/headless/lib/browser/headle...
headless/lib/browser/headless_devtools_client_impl.cc:135: NOTREACHED() <<
"Replay has neither result nor error";
On 2016/11/29 17:43:27, altimin wrote:
> nit: reply?

Done.

[altimin, 2016-11-30 12:12:16]

lgtm

[tmarek, 2016-11-30 13:02:29]

The CQ bit was checked by tmarek@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 13:02:41]

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, 2016-11-30 13:10:39]

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

[commit-bot: I haz the power, 2016-11-30 13:10:40]

Dry run: This issue passed the CQ dry run.

[tmarek, 2016-11-30 13:23:07]

The CQ bit was checked by tmarek@google.com

[commit-bot: I haz the power, 2016-11-30 13:23:28]

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-11-30 13:26:57]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480512187618730,
"parent_rev": "a6afc354ea7e7cf3dd3aadac8b82986b888a1cc0", "commit_rev":
"452ca2031b0bed1fda0f8fc036e6ec929daeb724"}

[commit-bot: I haz the power, 2016-11-30 13:27:20]

Description was changed from

==========
If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082
==========

to

==========
If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082
==========

[commit-bot: I haz the power, 2016-11-30 13:27:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-30 13:30:56]

Description was changed from

==========
If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082
==========

to

==========
If there is an error in a devtools API call, invoke the callback with a
TYPE_NULL value result and log the error message.

This is a partial fix because it at least allows detecting that there
was an error, while keeping source-code-level backwards compatibility.

BUG=651082

Committed: https://crrev.com/0c99aca28e9e59194c153ea13b85c1d7763467fb
Cr-Commit-Position: refs/heads/master@{#435240}
==========

[commit-bot: I haz the power, 2016-11-30 13:30:57]

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