New issue
Advanced search Search tips

Issue 848849 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 21
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

[DevTools] use extends instead of implements for protocol dispatchers

Project Member Reported by kozy@chromium.org, Jun 1 2018

Issue description

To make type checks using closure we generate protocol_externs, for
each domain we generate Protocol.${domainName}Dispatcher @interface.

SDK.${domainName}Dispatcher uses @implements with generated interface,
closure compiler in this case forces us to have all interface method
implemented.

Problem: when we try to add new event for one of V8 domains, we can not
add this notification and its implementation in the same CL, so closure
blocks landing new events in V8 domains.

We can resolve this problem by using @extends instead of @implements
for V8 domains. In this case closure will still check that if we
override something - we override it properly but allow us to not
override all events.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/14157eee77965e191ec21af278bdcb2aecf9f08e

commit 14157eee77965e191ec21af278bdcb2aecf9f08e
Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org>
Date: Fri Jun 01 22:18:50 2018

[DevTools] use @extends instead of @implements for RuntimeDispatcher

To make type checks using closure we generate protocol_externs, for
each domain we generate Protocol.${domainName}Dispatcher @interface.

SDK.${domainName}Dispatcher uses @implements with generated interface,
closure compiler in this case forces us to have all interface method
implemented.

Problem: when we try to add new event for one of V8 domains, we can not
add this notification and its implementation in the same CL, so closure
blocks landing new events in V8 domains.

We can resolve this problem by using @extends instead of @implements
for V8 domains. In this case closure will still check that if we
override something - we override it properly but allow us to not
override all events.

I tried this approach for Page domain as well since it contains a lot
of unused by DevTools events but my attempt failed because we have
SDK.ScreenCaptureModel. It implements dispatcher and extends model,
unfortunately closure does not support multiple inheritance and to
use @extends here we should extract dispatcher from ScreenCaptureModel.

R=dgozman@chromium.org

Bug:  chromium:848849 
Change-Id: I14fa87b53aeaff5bd7daea7dd36a6917688841cf
Reviewed-on: https://chromium-review.googlesource.com/1082594
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Andrey Lushnikov <lushnikov@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563830}
[modify] https://crrev.com/14157eee77965e191ec21af278bdcb2aecf9f08e/third_party/blink/renderer/devtools/front_end/sdk/DebuggerModel.js
[modify] https://crrev.com/14157eee77965e191ec21af278bdcb2aecf9f08e/third_party/blink/renderer/devtools/front_end/sdk/HeapProfilerModel.js
[modify] https://crrev.com/14157eee77965e191ec21af278bdcb2aecf9f08e/third_party/blink/renderer/devtools/front_end/sdk/RuntimeModel.js
[modify] https://crrev.com/14157eee77965e191ec21af278bdcb2aecf9f08e/third_party/blink/renderer/devtools/scripts/build/generate_protocol_externs.py

Status: Fixed (was: Assigned)

Sign in to add a comment