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>
The component server needs to inject CSS and JS scripts into each
example. It was doing this by relying on a <style> and <script> tag as
the hook to inject more code. But if you have an example without a
<style> or <script> tag, it won't work. Instead we now inject based on
the </head> and </body> tags, which will always exist (or, if they
don't, we have bigger problems!)
Bug: None
Change-Id: Id2f586c917ee7ab0821d5ab8e50e5cd3d3647a92
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2815131
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
The separate *_OWNERS files are related and can be grouped together
in a folder to make this distinction clear. This will allow us
to update the PRESUBMIT script to explicitly mark them as
exclusive change directory.
Note that the directory is named "owner", as we can't name it
"owners" as that is case-insensitive equivalent to "OWNERS",
which Windows can't handle in its presubmits.
DISABLE_THIRD_PARTY_CHECK=OWNERS update
R=yangguo@chromium.org
Bug: 1187573
Change-Id: Ib94a544c9dc01f788a8b75a5c8111ab52fbc27b8
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2814658
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
This CL adds a rule that ensures that any LitHtml.render call is
nested within a coordinator.write. It uses a fairly simple approach of
looking for a parent coordinator.write() call, but it should pick up most
cases.
Note this CL doesn't enable the rule; I'll do that in a follow-up. There
are 22 violations of the rule, so I will work on fixing those.
Bug: 1188116
Change-Id: Id73eaaa76db4685450086d2f77e7b2de322210d0
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2799754
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Prior to this CL, if we gave the dark mode generation script a CSS
file that contained:
```
.foo {
color: var(--override-some-custom-color);
}
```
The script would generate the following CSS:
```
.-theme-with-dark-background .foo,
:host-context(.-theme-with-dark-background) .foo {
color: var(--override-some-custom-color);
}
```
But because the value of `color` is a variable, this CSS does nothing
that the original CSS won't do. Therefore the CL checks each rule for
any declarations that include `var(--` in them, and removes them from
the CSS that is sent to be dark patched.
Bug: chromium:1188524
Change-Id: I1257ac7c8b9f11426ce5c9b44bd89900072cf1b3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2797199
Auto-Submit: Jack Franklin <jacktfranklin@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
This CL fixes a bug where the test runner took the `--chrome-features`
flag but Conductor never read that value when booting up a browser
instance.
This CL updates it so it will read the value and pass it into the
relevant argument when executing `puppeteer.launch`.
I also changed the Python test suite runner to not prepend
`--enable-features`; that feels like something that Conductor should
do, not the test runner.
Bug: chromium:1186163
Change-Id: I05916d39d60bb98a606570cd82e688581dd78a40
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2790874
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
This reverts commit 7742a201a1.
Reason for revert: We do have a use case for this flag that we missed, so we need it. Will follow-up with a CL to fix the fact that the test frontend infra doesn't use it currently.
Original change's description:
> [TestRunner] remove --chrome-features flag
>
> The scripts (both the old Python one and the new JS one) took
> `--chrome-features` as a flag, which was then set as
> `process.env.CHROME_FEATURES`, however searching the codebase for
> `CHROME_FEATURES` revealed that whilst we set this value, we never
> read it. Therefore this CL removes it entirely, my logic being that if
> someone needed this flag to work, we would have found this bug a long
> time ago.
>
> We can reintroduce should we find a usecase for it in the future.
>
> Bug: chromium:1186163
> Change-Id: I11d900df4aaad232fd64b9edca2aac12c94fc988
> Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2782555
> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
> Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
> Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Bug: chromium:1186163
Change-Id: I336a4a14a3e720f3b0626d78148038ceab9a8774
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2786942
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
In release mode, the devtools_entrypoint prebundle step was missing
the deps. We ran into this issue when we were moving the emulation
panel into panels/emulation, which required some updates to the
toolbox.ts entrypoint (crrev.com/c/2782540).
The missing deps then required the rootdir logic to be changed,
such that deps are correctly resolved relative to the gen-directory,
not the source directory.
This CL might improve build performance in release builds, since
we are now properly using incremental references for our
entrypoints and saving a bit of compilation time.
DISABLE_THIRD_PARTY_CHECK=TypeScript fix
R=aerotwist@chromium.org
Bug: 1187573
Change-Id: I4662ac90ba50116cb668dfe3164fe725b74f9a37
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2782554
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
The scripts (both the old Python one and the new JS one) took
`--chrome-features` as a flag, which was then set as
`process.env.CHROME_FEATURES`, however searching the codebase for
`CHROME_FEATURES` revealed that whilst we set this value, we never
read it. Therefore this CL removes it entirely, my logic being that if
someone needed this flag to work, we would have found this bug a long
time ago.
We can reintroduce should we find a usecase for it in the future.
Bug: chromium:1186163
Change-Id: I11d900df4aaad232fd64b9edca2aac12c94fc988
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2782555
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
The dark mode generation was disabled due to a bug
(https://crbug.com/1185185) in the Sources Panel.
The source of that bug was that the dark mode generation takes all the
CSS in the source CSS file, applies the colour patching to it,
prepends each rule with `.-theme-with-dark-background` and then writes
the new file to disk.
The problem was that any CSS overrides defined in the source file
would not be applied, because the automatic CSS generated by the dark
mode script would be loaded after it and therefore take precedent. The
fix is to make the script smarter and recognise any overrides provided
in the source CSS file, and include them in the bottom of the dark
mode script, so that they are still applied and override the generated
CSS. This isn't ideal, as it means we load some blocks of CSS twice,
but it's better than having to manually update colours in some of our
CSS files that have 50+ declarations to update.
This CL doesn't re-enable the use of any of the dark mode scripts,
this will be done in a follow-up.
Bug: chromium:1188524
Change-Id: I03a1daa57d5535bb768696ca0d7bafc968618035
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2780804
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
When source code is in nested folders, the es_modules_import rule
would incorrectly compute the containing folders from `front_end`.
Thus, if you would have two sibling folders in `front_end/panels`,
the rule would incorrectly determine that they are in the same
folder, while technically they are in a different folder.
Therefore, the logic is rewritten to compute the common prefix
of the two paths. If either one path ends up resolving to just
a file, we assume that they were both part of the same folder.
In other words: iff after removing the common prefix you still
are importing both paths across a folder boundary, they are
considered a different folder.
R=jacktfranklin@chromium.org
Bug: 1187573
Change-Id: Ib34583e258e005a124735e17e6a92e829eb37ef3
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2773054
Auto-Submit: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
Reviewed-by: Jack Franklin <jacktfranklin@chromium.org>
Tim van der Lippe