To make sure that a particular switch statement implements all
branches of, for example, a union or enum, we can implement the
`assertNever` function. This function will ensure in the type
system that the relevant branches are covered and will result
in compilation errors if a branch is missing.
This `assertNever` call is used together with a refactoring of
FormatterActions (which is a inlineable const enum) to ensure
that the tasks posted to the FormatterWorker stay in sync between
the formatter_worker and the FormatterWorkerPool. Since we can
not import the `formatter_worker` bundle in a non-worker context,
we instead extract it into a separate entrypoint, imported by both.
R=jacktfranklin@chromium.org
Bug: 1158395
Change-Id: Id643486936e7fad300d0f40774ee851adcc76ef5
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2589953
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>
For our unit tests, we couldn't import the entrypoint of the
formatter_worker, since that entrypoint assumed it was running
in a worker context. Since our Karma unit tests run in a browser
context, it would start throwing errors on the postMessage call.
Instead, we should separate out the concerns of the formatter_worker
implementation and integration of its worker context with the
FormatterPool. To that end, we separate the implementation bundle
from the worker integration into a separate file "formatter_worker-entrypoint.ts".
To ensure we retain full test coverage, the integration part is
tested through e2e-tests, while the implementation of the
formatter_worker will remain being tested by unit tests.
After this change, we will now also compute full code coverage for
the formatter_worker folder, which we couldn't do before.
All of this will be used to port CSS pretty-printing tests from
layout to Karma tests, which will require yet another implementation
detail of the formatter_worker, which will now be exported through
its bundle entrypoint.
R=aerotwist@chromium.org
Bug: 1024752
Change-Id: I5f10a3ebab10e1012cf2244df779cdc31da06ed7
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2575091
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
This was causing problems when trying to write a unit test that
included a file which transitively included formatter.js, which was
instantiating a SourceFormatter, leading to a chain of unfortunate
events which relied on some other module already being loaded, which
it wasn't.
To avoid the dependency chain between the imports, expose a function
getSourceFormatter which lazily instantiates a shared instance.
Bug: 1136848
Change-Id: I7c255186e53e131127ba00b7f719f2f1b34301c2
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2456592
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
The FormatterWorkerPool._onWorkerMessage method can call its current
task's callback with null as its parameter.
This happen when the formatter work doesn't send event data back.
When this happens, it creates exceptions in the ScriptFormatter class.
So, this change simply guards against this.
Bug: 1132284
Change-Id: I9a995a2b81b5e211ae1690caad7bf56a4772c354
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2431404
Reviewed-by: Jose Leal <joselea@microsoft.com>
Reviewed-by: Mathias Bynens <mathias@chromium.org>
Commit-Queue: Patrick Brosset <patrick.brosset@microsoft.com>
Since formatter_worker.ts is the actual entrypoint for the worker,
we should perform the message handling and postMessage invocations
there. This allows us to remove accesses to the `self.postMessage`
global in the implementation of the parsers and formatters, allowing
proper unit testing. Note all `self.postMessage`s have been removed,
as that requires additional refactoring in a follow-up CL.
This also paves the way for removing `formatter_worker_entrypoint`,
which is currently a no-op file. That requires additional infrastructure
changes to `Common.Worker` to allow specify a subfolder of `front_end/`
to be passed as the entrypoint.
Note that this CL also removes the `parseSCSS` method, which appears to
be unused in both the Chromium codebase as well as externally. Testing
on stable shows that we are currently not formatting `.scss` files
anyways.
R=aerotwist@chromium.org
Bug: 1011811
Change-Id: I808c5ea83efa5ec9fed6bd1aec2418487f553848
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2410232
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
This CL fixes a regression introduced in Chrome 61(!) where breakpoints
in pretty-printed code appear to persist after reloading the DevTools,
but don't actually function. The issue was that the source formatter
added its UISourceCode containing the formatted code to the project
before storing the metadata that allows its source mapping to work, so
the breakpoints for the pretty-printed code would be set on the
formatted script rather than mapped to a location in the actual script.
Video of bug: https://i.imgur.com/OTDlq5g.mp4
Same thing with the fix: https://i.imgur.com/AZ7nT17.mp4
Bug: 930334
Change-Id: I53830839ed45a78d11ad2cc27fc5cc1bf09ec4a4
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2368060
Reviewed-by: Jose Leal <joselea@microsoft.com>
Commit-Queue: Jack Lynch <jalyn@microsoft.com>
We should not be parsing our own JSON. Instead, we should be using
`JSON.parse`. However, `JSON.parse` is of course spec-compliant and
not "relaxed". Thus, it would fail on invalid JSON. But, we do show
invalid JSON as plain text. E.g., if you were to inspect an invalid
JSON response in the network tab and it is actually not valid JSON,
we don't show the fancy object-tree-view, but instead the raw content.
Given that having our own JSON parser is not ideal and we still
gracefully fall back to a usable JSON preview, we should remove the
RelaxedJSONParser.
After this change, we can handle valid JSON, valid JSONP and show a text
preview for invalid JSON(P).
R=aerotwist@chromium.org
Bug: 1098730
Change-Id: I3c0513518e36b29f35124b2810a119fd3828ff93
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2264560
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Paul Lewis <aerotwist@chromium.org>
This rule disallows assigning a value in a return statement.
It also works in one-liner arrow functions.
For instance, a => a = 2 will trigger a warning.
This is to avoid cases where you actually meant to compare a to 2, not
assign the value 2 (maybe you wanted to return a boolean in a filter
callback).
These sorts of issues can easily pass review and cause weird problems
that are hard to track down later.
The rule is enabled in this CL, and all warnings are fixed with this CL.
More info about the rule: https://eslint.org/docs/rules/no-return-assign
Bug: 1093379
Change-Id: I38b98ea59ecaedc83a7f69073b17980808f9a3bf
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2242761
Reviewed-by: Patrick Brosset <patrick.brosset@microsoft.com>
Reviewed-by: Jose Leal <joselea@microsoft.com>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Patrick Brosset <patrick.brosset@microsoft.com>
This patch makes it so that all module.json files use the same
consistent indentation that matches .editorconfig and the rest
of the codebase. At the request of reviewers, one such file
(front_end/emulated_devices/module.json) has been excluded
from this patch. It will be reformatted in a separate CL.
Note that this patch does not “roll CodeMirror”, although a
pre-commit hook forces me to include that phrase in the commit
message because the module.json file in the relevant folder is
modified.
Bug: chromium:1070492
Change-Id: Ib31ed5232461e2f80bf05f31105fc919f6639b0a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2127007
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Reviewed-by: Simon Zünd <szuend@chromium.org>
This CL changes references to self.Bindings.cssWorkspaceBinding (the
globalinstance of Bindings.CSSWorkspaceBinding.CSSWorkspaceBinding) over
to Bindings.CSSWorkspaceBinding.CSSWorkspaceBinding.instance(). To keep
both TypeScript and Closure happy we must make a method on the
CSSWorkspaceBinding class itself, since it only allows private
constructors to be accessed by static methods on the class.
Bug: 1058320
Change-Id: Ic42d2b76e5bcec38029827dce14e3bf06ceaf89a
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2107520
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
This CL changes references to self.Common.settings (the global
instance of SDK.Common.Settings) over to
Common.Settings.Settings.instance(). To keep both TypeScript and
Closure happy we must make a method on the Settings class itself,
since it only allows private constructors to be accessed by static
methods on the class.
Bug: 1058320
Change-Id: I04afc8caf64acf29cdda13ef03ad05cfff4786a1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2091450
Commit-Queue: Paul Lewis <aerotwist@chromium.org>
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
This CL is part of the cleanup effort to get rid of prototype
wrangling in utilities.js.
Note that {keysArray} on Set/Map can't be removed yet, as there are
a few use-sites in upstream chromium web tests that need to be cleaned
up first.
Bug: chromium:1050549
Change-Id: I3d5cdc9211763f53d0d43a110a67bba9485d0a11
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2046555
Reviewed-by: Tim van der Lippe <tvanderlippe@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Tim van der Lippe