headless: Implement DevTools events

headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353
Committed: https://crrev.com/8a15bb3cc46b4d9de5e7334e2746f993941f8a96
Cr-Commit-Position: refs/heads/master@{#389074}

[Sami, 2016-04-20 17:20:45]

Description was changed from

==========
headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353
==========

to

==========
headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353
==========

[Sami, 2016-04-20 17:20:45]

skyostil@chromium.org changed reviewers:
+ altimin@chromium.org

[Sami, 2016-04-21 13:27:23]

+dgozman

[dgozman, 2016-04-21 17:21:36]

dgozman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2016-04-21 17:21:37]

Code lgtm, see my comment about future compatibility.

https://codereview.chromium.org/1907533002/diff/1/headless/lib/browser/domain...
File headless/lib/browser/domain_cc.template (right):

https://codereview.chromium.org/1907533002/diff/1/headless/lib/browser/domain...
headless/lib/browser/domain_cc.template:118: FOR_EACH_OBSERVER(Observer,
observers_, On{{event.name | to_title_case}}());
It could be easier for clients to roll new versions of protocol if every event
has params. This makes signature of event method stable and adding (yet unused)
parameter to event won't break existing code.


Similar idea applies to generated shorthands for protocol methods. If domain or
method is hidden, it could make sense to only supply signature with a params
object to make it stable.

[Sami, 2016-04-22 10:36:22]

https://codereview.chromium.org/1907533002/diff/1/headless/lib/browser/domain...
File headless/lib/browser/domain_cc.template (right):

https://codereview.chromium.org/1907533002/diff/1/headless/lib/browser/domain...
headless/lib/browser/domain_cc.template:118: FOR_EACH_OBSERVER(Observer,
observers_, On{{event.name | to_title_case}}());
On 2016/04/21 17:21:36, dgozman wrote:
> It could be easier for clients to roll new versions of protocol if every event
> has params. This makes signature of event method stable and adding (yet
unused)
> parameter to event won't break existing code.
> 
> 
> Similar idea applies to generated shorthands for protocol methods. If domain
or
> method is hidden, it could make sense to only supply signature with a params
> object to make it stable.

Great points, I didn't consider that we might add new parameters to events that
currently don't have any. I'll implement your second suggestion in the patch
that adds experimental commands.

[Sami, 2016-04-22 10:36:27]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2016-04-22 10:36:27]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/1907533002/#ps20001
(title: "Add parameters to all events")

[commit-bot: I haz the power, 2016-04-22 10:36:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1907533002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1907533002/20001

[commit-bot: I haz the power, 2016-04-22 11:20:04]

Description was changed from

==========
headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353
==========

to

==========
headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353
==========

[commit-bot: I haz the power, 2016-04-22 11:20:06]

Committed patchset #2 (id:20001)

[altimin, 2016-04-22 16:01:24]

lgtm

[commit-bot: I haz the power, 2016-04-22 19:46:12]

Description was changed from

==========
headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353
==========

to

==========
headless: Implement DevTools events

Make it possible to subscribe to DevTools events from all domains.

BUG=595353

Committed: https://crrev.com/8a15bb3cc46b4d9de5e7334e2746f993941f8a96
Cr-Commit-Position: refs/heads/master@{#389074}
==========

[commit-bot: I haz the power, 2016-04-22 19:46:13]

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