Some files currently rely on the Protocol to be available on the
global scope. However, the Protocol definitions are defined in
a .d.ts file, which isn't available on runtime. Therefore,
attempting to import Protocol with non-type imports would retain
the imports in the `.js` files and break on runtime.
Since the only usages of the Protocol on runtime are the enums,
we can make them const, such that they get inlined as intended.
Then, `import * as` will work again, as the enums are inlined and
the import is removed from the `.js` file.
DISABLE_THIRD_PARTY_CHECK=Protocol update
R=jacktfranklin@chromium.org
Bug: 1208357
Change-Id: I749e57c9f51596866b61cab686c59f00bc8a8eb4
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2897277
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
In the Chromium backend engineers can use `DCHECK()` to ensure
certain invariants are held, but only execute these assertions
in a debug build. [1] Release builds do not ship with DCHECKS enabled.
In a similar fashion, introduce a build-time generated function
`DHCECK` that is only generated when `devtools_dcheck_always_on`
is set as GN arg. By default, `is_debug` builds enable
`devtools_dcheck_always_on`. However, in a release build, you can
explicitly set `devtools_dcheck_always_on` to `true` to achieve
the same effect.
When the GN arg is set, the build generates the `dcheck.js` file
with an implementation that checks the condition and fails if
it is not met. When the arg is not set, the function implementation
remains empty and becomes a noop.
To make sure that these functions calls are removed in a release
build (rather than being a noop), the terser configuration is
updated to treat these functions as pure. As such, terser will
remove any calls if the function implementation is empty. In a
release build that explicitly turns out the dchecks, terser
will not remove the function calls.
Lastly, to make sure that all code related to the dcheck is removed,
the condition needs to be a lambda. If we were to make it a raw
boolean, then `terser` would not be able to determine whether it
can remove the condition itself and would leave that behind. In other
words, the `DCHECK` call would still leave some artifacts behind,
namely the condition computation itself. By making it a lambda,
terser can deduce that the lambda creation has no side-effect and
remove the lambda if the `DHCECK` call is removed.
R=aerotwist@chromium.org
[1]: https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/c++/c++.md#check_dcheck_and-notreached
Bug: none
Change-Id: Ic396f102141d9eb67c8690bd2a601b56061b9d8c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2894390
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This CL lints that for a given component that all the references to
its tag name are the same.
```
class Foo extends HTMLElement {
// Check that this name
static litTagName = LitHtml.literal\`devtools-foo\`
}
// And this name
ComponentHelpers.CustomElements.defineComponent('devtools-foo', Foo);
declare global {
interface HTMLElementTagNameMap {
// And this one are the same
'devtools-foo': Foo
}
}
```
Bug: 1153077
Change-Id: I29694449cb37950d1a5ff5391779e16e462926e2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2897279
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This CL updates our ESLint rule for custom event names:
* It changes it from wanting kebab-case to allonewordnopunctuation
* It allows references to CustomEvent.eventName for times when it's
useful to define the event name as a static.
The CL therefore updates a variety of events through the codebase that
were kebab-case. go/building-ui-devtools has also been updated.
Fixed: 1176758
Change-Id: Ifbe9851bc2f6bbe9347ec886cc8c026248cc5c43
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2894389
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Currently, all Protocol type definitions live on the global scope.
Additionally, the protocol files are included in all ts_library
targets. However, we don't want the protocol definitions to be
available in, for example, reusable UI components.
Therefore, we should move to a system where all files that want
to refer to the protocol types should import them instead.
However, doing so in 1 large CL will be problematic, which is
why it should be both globally available and importable as an
interim step.
To do so, we augment the existing protocol definitions to export
them as namespace and regular export. Then, we introduce a separate
file that imports the protocol types and augments the global scope
with the definitions. Now, protocol is both importable and remains
available on the global scope.
The reason that we need a separate file is that TypeScript disallows
you to augment the global scope in a file that also exports types.
Therefore, the global scope augmentation happens in protocol-globals.d.ts,
which will be removed once all Protocol type usages are imported.
To verify that this approach works, ProtocolClient imports the
required types, while SDK only imports it in AccessibilityModel.
All other files in SDK still refer to the global type.
In follow-up CLs, all pre-existing usages of Protocol will use
the import style.
DISABLE_THIRD_PARTY_CHECK=Updating protocol type format
R=szuend@chromium.org,jacktfranklin@chromium.org
Bug: 1208357
Change-Id: I1d75949b9cd3e37989c6cddf79ac849f5664a1e3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2891756
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This CL adds an ESLint rule that bans the use of:
```
LitHtml.html`<devtools-foo>`
```
Because from now on we want to enforce:
```
LitHtml.html`<${Foo.litTagName}>`
```
I have disabled the rule in all locations where we do not yet do this,
and will be working to fix these problems over a series of CLs.
Bug: 1153077
Change-Id: I8d18243d0243ea1403d5d57dbeb32c8a9682d2dd
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2876969
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This script can be invoked to obtain the coverage from the unit
and interaction tests and writes them to a coverage-summary.json
file that lives in `test/`. This file can then be used in favor of
karma-coverage/coverage-summary.json to upload to the Chromium
infrastructure.
As a follow-up, we should turn on coverage collection on CQ and
hook up this script after both unit and interaction tests have
completed.
R=jacktfranklin@chromium.orgCC=liviurau@chromium.org
Bug: 1206705
Change-Id: I52199940d6747ec13f208233e04dcf81a618a8fb
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2884240
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
All interaction tests are now lazily instrumented with Istanbul
to obtain code coverage. The interactions tests can be started
with `COVERAGE=1` to obtain coverage. For that, the Mocha hooks
perform the eventual reporting and gathering of data. The instrumentation
is performed in the components server itself.
To make sure that we perform the minimal amount of work required
(since code coverage instrumentation is computationally expensive),
we preload pages to populate the instrumentation cache. Every
interactions tests should preload an example (most likely basic.html)
to populate the cache. Every subsequent test will then use the
already-instrumented code, rather than computing the code over
and over again.
The eventual code coverage is written to /interactions-coverage.
The results will eventually be merged with /karma-coverage
to obtain the union of both unit and interaction tests coverage.
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1206705
Change-Id: I5e19b1ecef23d21107210699cb29800556e0415e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2879986
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Prior to this fix, the wasmparser_worker entrypoint would include
specific files from third_party/wasmparser. However, since the
files were included as part of a separate package, rolling up the
target could cause issues. They should have been part of the
`sources` of the wasmparser_worker entrypoint, but they were instead
included as `deps`. Even better would be to not include specific
files from the wasmparser and instead use an entrypoint.
In the specific case reported in crbug.com/1203165, the wasmparser
implementation was updated. As such, GN reran
`third_party/wasmparser` and determined that
`entrypoints/wasmparser_worker:wasmparser_worker` required
recompilation (since one if its dependencies were updated. However,
since `entrypoints/wasmparser_worker:wasmparser_worker` wasn't
producing a different output, GN would determine that it wouldn't
have to run rollup. This conclusion is wrong and is an artifact of
the inclusion of specific files of `third_party/wasmparser` by
the entrypoint.
To fix this, we should rollup all relevant sources in
`third_party/wasmparser` instead. That way, whenever the wasmparser
implementation is updated, it will properly roll up its content
into its bundle, ready for consumption by the entrypoint.
R=jacktfranklin@chromium.org
Bug: 1203165
Change-Id: Ic29ddea0d1f8e953e11e71a6a0e4e65c5f0f1ad6
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2853559
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This locks down the visibility of ui/legacy:bundle to the various
folders that already depend on it. For now, the visibility is
quite broad, as there is still plenty of code depending on the
legacy UI implementation.
Consequently, downstream projects (such as the Edge DevTools fork)
will break if they depend on this bundle. Therefore, add a GN arg
that allows the visibility to be extended. To use this GN arg,
downstream projects can change their `default_args` in the root
`.gn` file:
default_args = {
devtools_ui_legacy_visibility = [
"//front_end/forked/folder/*",
]
}
This means that they can broaden the visibility of UI. It is still
recommended to remove as many of the dependencies on UI as feasible,
but that will likely not finish any time soon.
R=jacktfranklin@chromium.org,aerotwist@chromium.org
Bug: 1202788
Change-Id: I868e88ee3b1c66dd7c79d30d07648a7d2828e8f2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2853551
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
This CL adds issues for the CORS error codes
InvalidAllowCredentials
PreflightInvalidAllowCredentials
This also adds the $host_port replacement for headers in .rawresponse
files. This is useful if a HTTP header need to whitelist an origin
(which includes the port). Since our test setup changes the port
we host on every time, this placehoder is used in
test/e2e/resources/issues/acac-invalid.rawresponse
Screenshot: https://imgur.com/a/6lhV7WB
Bug: chromium:1141824
Change-Id: I9fb4a944241a5479b55cf5f97d46001346bb8a26
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2846328
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Wolfgang Beyer <wolfi@chromium.org>
GN metadata allows to specify and later query metadata from any
action in our build graph. Using GN metadata, we can specify
GRD files on an action and collect these files on the top level.
Then, we can compare the list of collected files to the expected
GRD files in `devtools_grd_files.gni` to ensure they match.
As a follow-up change, we can remove the intermediate lists we have
been specifying in `all_devtools_modules.gni` and
`devtools_module_entrypoints.gni`, which now become obsolete. That's
because both `devtools_module` and `devtools_entrypoint` now specify
the files in their respective metadata and essentially perform the
check that all relevant files are collected.
R=aerotwist@chromium.org
Also-By: alexrudenko@chromium.org
Bug: 1174013
Change-Id: I9dd2e6f7e010b5c25f83556511af12d5d1c2ec7c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2843322
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
I use the standalone devtools-frontend checkout process like most people
working on devtools, however I placed this repo inside chromium here:
D:\dev\chromium\devtools\devtools-frontend
This way I have both src and devtools-frontend side by side.
I just realized that this prevented me from running the css linter.
Indeed, run_lint_check_css.js tries to find the path to stylelint exe
file based on whether devtools is standalone or integrated in
chromium.src.
If it sees a directory named chromium inside the path, it assumes
devtools is integrated in chromium.src which, in my case, is wrong.
The change attempts to make this logic a little bit more safe by
checking a longer part of the path.
This resolves my issue.
Bug: 1198532
Change-Id: I872429da0751280bef5a4ec68602001421f55056
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2821855
Reviewed-by: Alex Rudenko <alexrudenko@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Brandon Walderman <brwalder@microsoft.com>
Commit-Queue: Patrick Brosset <patrick.brosset@microsoft.com>
This CL fixes the fact that the stylelint rule wouldn't deal with:
```
border-bottom: var(--foo) solid var(--color-details-hairline)
```
It does this via a naive regex that splits the border value into its
three pieces, and then only lints the final declaration, which is the
color.
Bug: 1198504
Change-Id: I409adaf8b8777112ce4f3b24f0a6f63bd7435c25
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2823831
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Tim van der Lippe