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>
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>
front_end/Runtime.js was doing multiple things: it was instantiating
side-effecty descriptors and initializing applications, as well as
storing all state for the modules/extensions, etc...
To support proper ES Modules, we need to separate these. root/Runtime.js
contains simple classes that store the data and can be retrieved, as well
as some helper functions that will load the respective data.
The RuntimeInstantiator.js code includes the start functions used in the
entrypoints themselves. They will use an instance of the Runtime to
properly load the application.
Along the way, I also fixed various TypeScript errors, mostly around
incorrect or missing type definitions.
R=aerotwist@chromium.org,jacktfranklin@chromium.org
Bug: 1011811
Change-Id: If3ddea9267813691b3faeb6ea4b0cf64edbde1f4
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2107523
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Settting the Ninja arg `is_debug` will retain the old behavior. However,
setting that flag to `false` will start using Rollup. Rollup will bundle
everything into an entrypoint, but will avoid bundling module
dependencies.
For example, if a file in `ui` depends on `common`, then everything in
`ui` will be bundled into `ui/ui.js`. But, it will retain all imports to
`common` as `import * as Common from '../common/common.js';`.
This also caught an issue with misconfiguration in the
wasmparser_worker, which should import the files directly and can't use
it in the dependencies array of the module.json
Lastly, the BUILD.gn definition is significantly cleaned up. In the
process of doing all this work, I discovered that we can simply use
`foreach` to iterate through all source files. This removes the
duplication of both `fron_end` and `$resources_out_dir` and
significantly reduces the amount of clutter in BUILD.gn.
As such, update check-gn.js to check for the correct GN variables.
roll CodeMirror: ignore
Bug: 1046596
Change-Id: I7dcda7949e79e001c8a11307dc902f96a8fa6696
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2089906
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
This checks in the InspectorBackendCommands in the source tree file
and ensures that it is kept in sync whenever the third_party
location has been updated.
In the process, I discovered multiple misconfigurations in the formatting
presubmit check. First of all, it was never running, because the
.eslintignore had an empty line. Second of all, it was running twice,
which is unnecessary since we now check for changed files at the end
of the presubmit. Lastly, it was only formatting JS files, while it
should check all files.
I have also updated the _CheckGeneratedFiles check to only run if it
is actually necessary. If there are no changes made to any affected of
the files, it will skip the step. This should thus reduce the presubmit
time and we will only pay the cost if we actually update any of the
files.
Lastly, it will now properly format and lint the generated files. This
makes reading the code a lot easier and makes it easier to digest the
diff when a protocol update goes through. I have verified that, in a
full build, the files are still minified. Thus, this has no impact on
the loading performance.
DISABLE_THIRD_PARTY_CHECK=Updating protocol generation
Fixed: 1056614
Change-Id: If49b0e749978ea1a7838992ec13507ee761ad76c
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2087765
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
This changes the Common.Worker to create Workers that use the module
scope rather than the global scope. It allows us to use es-imports,
which consequently allows us to remove the bundling during the build
step. This should hopefully improve the Ninja non-determinism that we
were facing when making changes to files that are used in the various
workers.
Bug: 1006759
Change-Id: I7ac209817b1576d2accd790f1bf7ef6a658f5605
Fixed: 1013129
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1981615
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Since workers do not support modules [1], it is not possible to
currently import any ESM DevTools module. To be able to migrate
text_utils/ to ESM, we would need to duplicate a large portion of
text_utils/, which would be unmaintainable.
We already had the same situation when we migrated platform/ to ESM and
decided to copy the relevant functions, but this time around that is no
longer an option.
Thus, the 2 workers (heap_snapshot_worker and formatter_worker) are now
being bundled on build time. This means that in a release build, the
respective entrypoints are bundled and inserted in the output.
To bundle, we use `rollup`, which is a bundler only concerned with
rolling up ES modules. All other functionality of rollup (such as
tree-shaking) is unused. We can revisit later if we need a bundler for
the rest of devtools, but since we are in active migration to ESM that
is infeasible at this point in time.
As part of this CL, the following folders are migrated to ESM:
- cm_headless/
- formatter_worker/
- heap_snapshot_model/
- heap_snapshot_worker/
- text_utils/
Since text_utils is also an autostart module for the shell, this is the
only module that is imported from root.js.
Some of these modules also include files that are annotated with
skip_compilation and thus run with dummy files during Closure
compilation.
Note that, because of the usages of import-statements in the workers
and the blocking bug [1], the workers are generated even when Chromium
is built with `debug_devtools=true`.
Since heap_snapshot_model/ also used in the profiler, we have to eagerly
load this module in root.js. Once all other dependencies of the profiler
have been migrated to ESM, we can remove this import-statement.
[1]: crbug.com/680046
Bug:1013129,1006759
Change-Id: I4c03c7b8a1f351ae9693cbcd922412083dd34bba
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1883707
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Any module that is not autostart and has `modules` specified in its `module.json`
will now have its entrypoint dynamically imported. Update the release build script
to output the `modules` array as well so that the Runtime can load it.
Add additional entrypoints to `platform` and `dom_extension` to be consistent
with the naming patterns of all other modules
Bug:1006759
Change-Id: If6d10a13a62354079e3f8ee49bee4ecdcffa6758
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/1893085
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
After converting the `ui/` folder to ESM, there was a significant performance
regression in terms of file size of the `resources.pak`. It turns out that we
were not minifying ESM files, but we did do that before with Closure.
Update the `copy_devtools_modules` to also invoke rjsmin to minify the
JavaScript source code.
Before:
$ du -h -B K out/Default/resources.pak
12096K out/Default/resources.pak
After:
$ du -h -B K out/Default/resources.pak
11752K out/Default/resources.pak
Before the original CL transforming `ui/` landed:
$ du -h -B K out/Default/resources.pak
11860K out/Default/resources.pak
This means that after converting `ui/` to ESM, we saved 90KB. We suspect
this is because we are no longer transpiling to ES5, but instead serve
source ES6.
Bug: 1010910
Change-Id: I7dd07ff788d014e9f9a8e25e168c58c010cd00af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1837814
Commit-Queue: Tim Van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#702500}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 8d2021d8394b9a248436fb6976070629cdffd961
Alex Rudenko