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>
We were using community-maintained for integrating Istanbul with
Karma. However, there appears to be an official karma-coverage
plugin that is maintained by the Karma team. Moreover, the latest
changes on that plugin have removed dependencies on packages
that have reported vulnerabilities.
Therefore, use karma-coverage and remove the old community-maintained
packages to significantly reduce the amount of vulnerable NPM
packages.
R=jacktfranklin@chromium.org
DISABLE_THIRD_PARTY_CHECK=Remove old community-maintained packages
Bug: 1068145
Change-Id: Ie81c185155db6598fc0cd05d9405670a0568c1c1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2140942
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Currently, ShortcutRegistry allows multiple shortcuts to be stored
within the same binding in a module.json, e.g. `"shortcut": "F8
Ctrl+\\"` creates two shortcuts, one on F8 and another on Ctrl+\. When
multi-keypress shortcuts/chords are added in the future, they'll use the
same format such that the above example would be interpreted as a single
shortcut bound to the F8 Ctrl+\ sequence. In order to allow that, it's
necessary to split up all of the existing shortcuts that are stored in
space-delimited strings so that they'll continue to function as
expected. This CL also updates the module.schema.json to disallow spaces
in shortcuts, a restriction that will be removed once multi-keypress
shortcuts are implemented.
Custom keyboard shortcuts design doc: https://docs.google.com/document/d/1oOPSWPxCHvMoBZ0Fw9jwFZt6gP4lrsrsl8DEAp-Hy7o/edit#heading=h.2xpjzz3fl1ju
Bug: 174309
Change-Id: I853f9918ad2892b2f4c4f3aec53013d7a6455f67
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2123807
Reviewed-by: Robert Paveza <Rob.Paveza@microsoft.com>
Reviewed-by: Vidal Diazleal <vidorteg@microsoft.com>
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
While clang-format and eslint support the modern javascript features,
the localizability pipeline did not. It was using esprima, which is
a parser that is not well maintained and does not support modern
features.
For our ESLint configuration, we are using the typescript-eslint
parser, which is compatible with espree, the parser powering ESLint.
Both espree and typescript-eslint have an API that is very similar
to esprima.
Therefore, update the localizability pipeline to use the espree API
with the concrete implementation of the API by typescript-eslint.
In the future, the localizability pipeline should probably be an
ESLint plugin so that we can unify the parsing experience.
A follow-up CL will remove all usages of esprima.
R=jacktfranklin@chromium.org
Fixed: 1068966
Change-Id: Idcffb8d649f006d7cf0b3de0ee0d886cb3848230
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2140939
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This allows us to execute the following:
npm run install-deps -- outdated
Which is equivalent to `npm outdated`, although it will run on the
actual node_modules output (that we can verify is unchanged).
Most of the npm commands require the private information to exist.
Therefore, if we run a custom command we first have to install with
`npm ci` to get the private information. Then we have to execute
the command, perform the cleanup and only after that finisht the
script. If we would bail out right after executing the custom command,
the private information would remain in the repository.
R=jacktfranklin@chromium.org
Fixed: 1068132
Change-Id: I50d3538a7115783dea19e1899faf17b1622f22ff
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2137381
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Lexical declarations in `case` and `default` clauses are a footgun,
since they are visible in the entire switch block, but they only
get initialized upon assignment, which only happens if the relevant
`case` is actually reached.
To ensure that such lexical declarations only apply to the current
`case` (which is usually the intention), `case` clauses containing
them should be wrapped in curly braces to create an explicit block.
More information:
https://eslint.org/docs/rules/no-case-declarations
Change-Id: I63d9341fcd76d4b9ce8281bd0e6573b886577f08
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2119685
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Commands, Events and Object types can declare "inline enums" to
restrict the possible values of a 'string' field.
Example field:
referrerPolicy: ('unsafe-url'|'...'|'...')
To enable type-checking with TypeScript and stay compatible with
existing code, we now generate explicit enums. The naming scheme
for the enum names is adapted from code_generator_frontend.py
and needs to always match.
Example generated enum for the above code:
export enum RequestReferrerPolicy {
UnsafeUrl = 'unsafe-url',
NoReferrerWhenDowngrade = 'no-referrer-when-downgrade',
NoReferrer = 'no-referrer',
Origin = 'origin',
OriginWhenCrossOrigin = 'origin-when-cross-origin',
SameOrigin = 'same-origin',
StrictOrigin = 'strict-origin',
StrictOriginWhenCrossOrigin = 'strict-origin-when-cross-origin',
}
This is necessary as we didn't had any type for this enum before
but existing code was using
Protocol.Network.RequestReferrerPolicy
as a type in JSDoc.
R=tvanderlippe@chromium.org
Bug: chromium:1011811
Change-Id: I4b4aa04b69fa4d7bf3b79ad97d61d4e3bfb7e228
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2113374
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Instead of
export type MixedContentType = ('blockable'|'...'|'none');
we know emit
export enum MixedContentType {
Blockable = 'blockable',
OptionallyBlockable = 'optionally-blockable',
None = 'none',
}
This is necessary as existing JavaScript code accesses these
enums using
Protocol.Security.MixedContentType.None
R=tvanderlippe@chromium.org
Bug: chromium:1011811
Change-Id: Id4bf2c1333affcdc4df37a43bf95379700e3e9d0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2113373
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Philip Pfaffe