This change updates the component bridge generation code to understand
object literals and interfaces within them, e.g:
```
set data(data: {x: MyInterface, y: SomeInterface | null})
```
The bridges code understands how to generate the correct `@param`
documentation for that setter, and is able to parse through the object
to find the use of the two interfaces to know that they are interfaces
we need to convert and output in Closure land.
Change-Id: I637309c73721b6a57cd5d06f09600d16bfd207aa
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2183912
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The `headlesscodemirror.js` was actually a copy of
`addon/runmode/runmode-standalone.js`. Using the third_party source
allows to remove configuration for clang-format, ESLint and the
roll_codemirror.py files.
The cm_headless module is used in the formatter_worker. I verified that
the import is working as intended by building DevTools and formatting
a CSS file (which is one of the formatters that makes use of CodeMirror).
Then I found out that the runmode-standalone file expects `window` to
exist, which doesn't exist in workers. Therefore, locally patch that
to `globalThis.CodeMirror`.
R=mathias@chromium.org
Bug: 1011811, 1076825
Change-Id: Id8e5ed1d0ca81c3699def73a0eb63835ffc7a43f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2174418
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
This CL adds the ability (hidden behind an experiment) for users to
select a set of shortcuts to use, the two options initially being the
default shortcuts and shortcuts that match those from VS Code. A new
`keybindSets` array is added to binding objects in module.jsons,
allowing shortcuts to be filtered on launch much like the existing
platform option. Also like platform, if the keybindSets array is
undefined for a shortcut then that shortcut will be considered common to
all keybindSets.
In general, I left keybindSets undefined for default DevTools shortcuts
that do not conflict with VS Code shortcuts so that they remain common
shortcuts. When both the DevTools and VS Code define the same shortcut,
I add both sets to keybindSets.
This should only affect users who have opted into the custom keyboard
shortcuts experiment. If a user with the experiment enabled selects VS
Code shortcuts and then turns the experiment off, ShortcutRegistry will
reset the shortcuts to the DevTools default on next launch.
Enumerating the differences between VS Code and the DevTools default
shortcuts uncovered some workflows that we'd like to add shortcuts for
that do not currently have actions in the DevTools, such as save as and
find/replace. I plan to add those in future CLs.
Due to problems with ListWidget's accessibility, I have to manually add
aria roles to the list and list items. This is a temporary state of
affairs until this tab is changed to use ListControl rather than
ListWidget for better accessibility. Since the ListWidget here contains
no editable items and is only marked as a list, there are no keyboard
navigation or focus management requirements to worry about yet.
I have put up a telemetry CL here so that we can evaluate the success of
the feature:
https://chromium-review.googlesource.com/c/chromium/src/+/2172132
Screenshots:
https://i.imgur.com/ka0VEyM.pnghttps://i.imgur.com/CG8hAro.png
Custom shortcuts design doc: https://docs.google.com/document/d/1oOPSWPxCHvMoBZ0Fw9jwFZt6gP4lrsrsl8DEAp-Hy7o/edit#
Bug: 174309
Change-Id: I0e3484eb2712c945121cc7b68e14a1a98f858bab
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2095859
Reviewed-by: John Emau <John.Emau@microsoft.com>
Reviewed-by: Songtao Xia <soxia@microsoft.com>
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
This CL adds a central singleton that collects all issues from all
known targers. This singleton is then used by the issues view to
display issues.
This change enables correct handling of issues that arise from OOPIF
and worker sources.
Fixed: chromium:1073797
Change-Id: I75a8bb46d240f63df013c0b06506cad7fb2e5a8f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2168885
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
Throughout this process we learned the following:
- The magically generated agents need to be properly exposed
on the target, rather than setting them on the TargetBase prototype.
- We need to rename the protocol proxy api definitions to use
the invoke_ naming, such that we can use structured request bodies.
This will allow us to no longer rely on parameter ordering and
does not require additional changes to the underlying Closure
generated code.
- Instead of using a symbol as an index on a different class, use
a WeakMap to keep track of the link between the NetworkRequest
and the NetworkManager. This breaks the circular dependency and
allows us to remove the lookup with the symbol
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1011811
Change-Id: I6cf25533b32793636d970b0a6c108f739d4e757e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2167868
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Even though we were using the SDK files in the unittests for SDK,
not all files were included. A CL which attempts to use sdk.js (e.g.
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165757)
would then fail on several generated .d.ts issues.
Issues fixed:
- Usage of `Object` instead of `Map` causing issues with a type
alias of `string`, since TypeScript object notation only allows
primitives to be used as indices.
- Missing return types for function types
- A reference to `Protocol.NetworkAgent` which does not exist on
the protocol. Instead, that is part of the protocol-proxy-api.
Therefore, we must include the declaration file in the ts_library.
This also showed that the `ProtocolApi` needs to be exported
instead of declared.
This means that for any future reference to any Protocol type
that is actually an agent, we should be using the
`ProtocolProxyApi` definitions instead. To make sure Closure
understands that type, I aliased it in the externs.
R=jacktfranklin@chromium.org
CC=sigurds@chromium.org,szuend@chromium.org
DISABLE_THIRD_PARTY_CHECK=Typescript fixes
No-Presubmit: true
Bug: 1011811
Change-Id: I4f5a488edb2d5fa6c5ed12d33411efb5f7fb8133
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165795
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Platform is the "lowest" part of DevTools - e.g. it depends on nothing.
We have code we want to move to Platform but cannot because it needs
`ls` and therefore creates a circular dependency. So this CL moves
UIString into Platform.
To avoid rewriting a lot of imports we re-export it from `common.js`.
Change-Id: I399bbe8593e043148c4c7e51f598cbd73a89f639
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165758
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
This patch includes some manual changes to the MediaModel, in order
to support the new playerMessagesLogged and playerErrorsRaised events
which are added in the roll.
The patch also includes the result of:
scripts/deps/roll_deps.py $CHROMIUM_SRC_DIR $DEVTOOLS_FRONTEND_DIR
As a drive-by, this patch also makes `scripts/deps/*.py` executable so
that they can be invoked directly without the need to explicitly invoke
Python.
DISABLE_THIRD_PARTY_CHECK=see above
No-Presubmit: true
Bug: chromium:1075437
Change-Id: I794006f5a2077c8929f7d28bdd3f7b308603b6d9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165756
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
This significantly speeds up Karma execution times to about 12 seconds on an unchanged build folder.
It uses the Ninja build output to find the unittest files.
You can run the new script with:
npm run unittest
npm run unittest -- --target=Release
To make sure you perform a minimal build and run tests right after it, run:
npm run auto-unittest
npm run auto-unittest -- --target=Release
If no ninja-build-name is set, it assumes that `out/Default` exists.
The `auto-unittest` command will run autoninja for you on the output folder.
R=jacktfranklin@chromium.org,aerotwist@chromium.org
No-Presubmit: true
Bug: 1061125
Change-Id: I45edd11e422c5cdc8a4fc0bbb6bc43e386519aa9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2102717
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Brandon Goddard <brgoddar@microsoft.com>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Previously, the script would output code of the form:
new Map(Object.entries({
'word-wrap': 'overflow-wrap'
}));
This patch simplifies that down to:
new Map([
['word-wrap', 'overflow-wrap']
]);
No-Presubmit: true
Bug: chromium:1039620, chromium:1075437
Change-Id: Ia102b46b70bdbb227c5ff41742fcb82940d48eb9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2165787
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
This CL adds the ability to bind actions to a sequence of two
keypresses, rather than just one (e.g. Ctrl+K Ctrl+O). VS Code refers to
these as chords [1]. As we move forward with custom keyboard shortcuts,
it's necessary to implement chords in the DevTools so that users
can match their shortcuts to editor shortcuts that use chords or assign
custom chords. This CL also adds a Ctrl+K Ctrl+S shortcut to open the
shortcuts settings for testing purposes, but I plan to remove that
shortcut before landing this change. It will return later as part of the
VS Code editor preset.
My implementation of chords here is focused on two-keypress
shortcuts, e.g. Ctrl+K Ctrl+Shift+Q would be a valid chord but Ctrl+K
Ctrl+K Ctrl+O would not be because it has three parts. Limiting chords
to two parts simplifies the implementation and matches a similar
restriction in VS Code. Although other editors like vim and Atom allow
shortcuts of arbitrary length, that's not really a feature that users
have asked for and it would introduce extra complexity into
ShortcutRegistry for questionable gain. However, the new structure of
ShortcutRegistry leaves open the possibility of enabling
arbitrary-length shortcuts in the future.
ShortcutRegistry's approach to handling a key has been changed as
follows:
If a keypress comprises only modifiers (e.g Ctrl+Shift), then it's
ignored. The DevTools already disallow modifier-only shortcuts, so this
change just prevents modifiers from clearing the chord timeout.
If the first half of a chord has been pressed within the timeout
(currently 1000ms), then clear the timeout and try to execute the
current key as the second half of a chord. If that isn't a valid chord,
then try to execute both keys as separate shortcuts in sequence.
If there isn't an active timeout and the keypress is potentially the
first part of a chord, then set _activePrefixKey and
_activePrefixTimeout. If the timeout expires without a second key being
pressed, attempt to handle the keypress as an individual shortcut.
If the keypress isn't potentialy the first part of a chord and there
isn't an active timeout, then it will be handled as normal.
There were a few shortcuts handled outside of ShortcutRegistry (e.g.
sources.rename, debugger.toggle-breakpoint) that made the assumption
that checking the key of a single event was enough to determine whether
it matched a shortcut, so that flow has been reworked to centralize all
shortcut-matching in
ShortcutRegistry.handleKey().
Custom shortcuts design doc: https://docs.google.com/document/d/1oOPSWPxCHvMoBZ0Fw9jwFZt6gP4lrsrsl8DEAp-Hy7o/edit
[1] https://code.visualstudio.com/docs/getstarted/keybindings#_keyboard-rules
Bug: 174309
Change-Id: I1b3f384d7c65e41d0dbc5e32854fb331e052823f
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2125620
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
This has two benefits:
1. We can now "pre-process" the arguments. It therefore allows us
to specifically set the tests we want to run. Before this change,
we would incorrectly include JavaScript files that were outputs
from (since-removed) TypeScript tests. Since adding these as
arguments to the Mocha invocation causes issues on Windows bots
with "arguments too long" errors, we should be using the config
file.
2. We can add additional arguments here without the need
of passing all the arguments from `run_test_suite.py` in.
R=petermarshall@chromium.orgTBR=aerotwist@chromium.org
Bug: 1071369
Change-Id: Icab02b1117f4095081987b65c8151ddf04239e11
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2157044
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
The shared test runner logic was reimplementing parts of Mocha,
in particular the test logging and filtering. Moreover, it was
booting the hosted mode server and puppeteer outside of Mocha.
Mocha supports root level hooks [1]. These hooks allow us to
perform work before a test starts. Moreover, by using the
before and after hook, we can run logic before and after
all tests.
By using these hooks, we can extract the "boot the hosted mode
server and hookup puppeteer" part to these root hooks.
Additionally, we can reset the pages in the `beforeEach`, which
means that tests themselves don't have to reset the pages.
We also put the implementation code into third_party/conductor,
as we would like to reuse this logic for the Puppeteer tests.
The Puppeteer test suite now also uses Mocha and has very
similar requirements as to our DevTools tests. By extracting
from DevTools, we can look into expanding the test runner to
other usecases, but that is out of scope for now.
[1]: https://mochajs.org/#root-level-hooks
Bug: 1071369
Change-Id: Ie9f954359d9de84da564b74b6f5517dd535db008
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2150458
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
I ran into this while trying to reproduce Closure failures locally for
a different CL. Python's built-in remove method on lists returns None
instead of the modified list, which was causing the resulting value in
exec_command to be incorrect when a Java install without a server JVM
was used.
Change-Id: Ib2629b513c2f6401c51654e68792509796366e61
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2150731
Reviewed-by: Leo Lee <leolee@microsoft.com>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Tony Ross <tross@microsoft.com>
We ignore some files for ESLint, as they are either generated or
third_party. However, when someone tries to change these files,
ESLint would emit a warning stating that the file in question is
ignored.
This issue was reported to ESLint in https://github.com/eslint/eslint/issues/9977
The suggested workaround is to use the CLIEngine to filter out
the problematic paths. However, since our script was written in
Python, that API is not accessible to us.
Therefore rewrite the script to Node and filter out the problematic
files. The calls to CLIEngine were mostly taken from eslint/lib/cli.js,
which was the previous file used by `node_modules/bin/eslint`.
R=jacktfranklin@chromium.orgCC=sigurds@chromium.org
Change-Id: Iee600f0e0d99fcb6eeeb203a952a50fe35f9aaf3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2149316
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Tim van der Lippe
