Constant Sets now have their own classes rather than being constructed as a wrapper over a const Map. The new scheme is similar to Maps - using a ConstantStringSet backed by a JavaScript Object 'dictionary' when the elements are all suitable String values, otherwise using a GeneralConstantSet backed by a list of elements that is converted to a lookup map on demand.
Constant Sets and Maps with suitable String keys are backed by a JavaScript Object literal 'dictionary'. The use of the Object literal has been changed so that the property values are no longer the values of the Dart Map, but the position of the key in the order of the entries. The values are provided as a List (Array), so there is an additional indexing operation to access the value on lookup. This does not seem to affect performance.
The advantage of the two-level lookup using the 'dictionary' is that Maps with the same keys (in the same order) can share a 'dictionary'. In order to achieve the advantage, the JavaScript Object is modeled as a first class ConstantValue - JavaScriptObjectConstantValue.
These changes achieve a code size benefit of -0.90% (~130K) on the main unit of a certain large app, and -0.35% for flute/benchmarks/lib/complex.dart
Change-Id: Icad2e6136218486a439e3c5ed0296462e3c3c4e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310020
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Revisions updated by `dart tools/rev_sdk_deps.dart`.
bazel_worker (c14a268..f7b714e):
f7b714e 2023-06-16 Jacob MacDonald add firehose publishing workflow (#76)
0743f11 2023-06-16 Parker Lougheed Require protobuf 3.0.0 and prepare for 1.0.3 release (#75)
dartdoc (5799424..793d575):
793d575d 2023-06-14 Sam Rawlins Remove unused coverage feature (#3444)
19ad21a4 2023-06-14 Parker Lougheed Replace `@sealed` meta usage with keyword (#3445)
8e9d89e3 2023-06-12 Sam Rawlins Remove all `@deprecated` elements. (#3250)
2019791c 2023-06-12 dependabot[bot] Bump actions/checkout from 3.5.2 to 3.5.3 (#3442)
d34c7067 2023-06-12 dependabot[bot] Bump github/codeql-action from 2.3.6 to 2.13.4 (#3443)
http (ba7eb60..8c25057):
8c25057 2023-06-16 Nate Bosch Unskip a skipped test (#966)
protobuf (a9bf79f..e76bd74):
e76bd74 2023-06-19 Ömer Sinan Ağacan Remove protoc_plugin exe from repo, update .gitignore (#851)
4b71e60 2023-06-16 Ömer Sinan Ağacan Fix a change in protoc_plugin changelog (#846)
f4ffa1d 2023-06-16 Ömer Sinan Ağacan Fix missing protobuf import in grpc files (#845)
source_maps (dd5b5cd..58eef30):
58eef30 2023-06-12 Kevin Moore Require Dart 3, update lints (#79)
tools (8d6e8b8..02b5cc5):
02b5cc5 2023-06-16 Brian Wilkerson Add documentation for server events (#115)
698bfe8 2023-06-15 Elias Yishak Introducing new `Event` class that can be send via `analytics.send(Event event)` (#114)
webdev (81ae77a..b58edb7):
b58edb7d 2023-06-16 Elliott Brooks Fix the DCM workflow (#2147)
1551fe6c 2023-06-16 Elliott Brooks Wrap VM service API methods in an error handler to convert any exceptions into type `RPCError`
(#2144)
06abe901 2023-06-14 Elliott Brooks Add more tests to daily testing cron job (#2138)
5cb2dcf6 2023-06-13 Elliott Brooks Use new error codes from package:vm_service (#2141)
Change-Id: I6779c6d20fbc69e83848f469df3e0b4d21420a9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310322
Auto-Submit: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
A lot of this change is just documentation comments, but it includes some minor updates such as supporting "lazy" on variables.
All changes are backwards compatible and this should not affect any tests or functionality of Dart's DAP implementation (but provides the ability for us to use the new features).
Change-Id: Ief81927f2f370b4bfb6b5a9acc8659c45c33ea18
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310161
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Sometimes compiler omits type checks for the 'await' and 'return from
async' by relying on the fact that certain classes cannot hold
instances of Future.
In JIT mode, the property 'class-can-be-Future' may change when a new
class is loaded dynamically (e.g. through IsolateMirror.loadUri).
In such a case, the generated code assuming that certain class
cannot be Future should be correctly deoptimized.
This CL implements this deoptimization by adding such classes into
the list of CHA guarded classes.
TEST=runtime/tests/vm/dart/await_type_check_with_dynamic_loading_test.dart
Change-Id: Ib3a3768d073e8562b794186e72cb9d61e9f496fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309822
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
When an isolate is paused, we store some data for things like variables, stack frames, etc. to round-trip an identifier to the client that it can ask about later.
Previously we never cleared these, so over time in a long debug session they could build up. The DAP spec says these references become invalid when execution is resumed:
> To simplify the management of object references in debug adapters, their lifetime is limited to the current suspended state. Once execution resumes, object references become invalid and DAP clients must not use them.
Because it's possible to resume one thread without another, I'm clearing up only the specific thread (isolate)s data when it resumes, rather than all of it.
Fixes https://github.com/Dart-Code/Dart-Code/issues/4420
Change-Id: I2ba7d9cd3f23b5a628d9e279e49edf2bee5afd29
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310342
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Any adb failures when test_runner runs tests on android devices,
except an failure of the actual precompiled test when it is run,
should be reported as an infra failure and should shut down the
test runner.
After change https://dart-review.googlesource.com/c/sdk/+/291304
these types of failure were reported as test failures instead.
Fixes: b/281492587
Change-Id: Iea47ccffac81b0b92f3a3fb3780d2181d08fc1d1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310260
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: William Hesse <whesse@google.com>
This reverts commit c30bec0f1a.
Reason for revert: Broke RISC V build
Original change's description:
> Update Binaryen to 4ec79ce9826d96225d33dec77dfc344adab92606
>
> The encoding for stringref instructions has been updated to match V8.
>
> This is the version right before the change to output the new
> encoding for `br_on_cast[_fail]` instructions with input immediate.
>
> Change-Id: I0ac1cb81e3529b7d903dbf7c6232383d6af46ed0
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309860
> Commit-Queue: Aske Simon Christensen <askesc@google.com>
> Reviewed-by: Ömer Ağacan <omersa@google.com>
Change-Id: Iea0f3ef76f98fbc9a2baed525060883e1aacb274
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310300
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Auto-Submit: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
This adds a check that all annotations of macro classes correspond
to applied macro applications. This should be improved in the future
to detect what the problem with the annotation was. For now it helps
avoid inadvertently using invalid/unrecognized annotations in the
macro feature development.
Change-Id: I9574e2e24150d2febfc5489ab4ebf0ae2627e3fd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310101
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Initial commit at https://dart-review.googlesource.com/c/sdk/+/276100
used kb because that's what was reported by the system.
I wasn't allowed to add that to golem though, and then I forgot about
it.
This CL changes it to bytes so it can hopefully by allowed into golem.
Change-Id: Ia7fae9ed47e6d237648e266c26f4a644f13571d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310160
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The encoding for stringref instructions has been updated to match V8.
This is the version right before the change to output the new
encoding for `br_on_cast[_fail]` instructions with input immediate.
Change-Id: I0ac1cb81e3529b7d903dbf7c6232383d6af46ed0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309860
Commit-Queue: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Ömer Ağacan <omersa@google.com>
This CL adds the token position of the checked parameter when doing the
AssertAssignable for better stacktraces when it fails.
E.g.
```
$ cat t.dart
class A {
void takesA(A a) {
print("Hello A");
}
}
class B extends A {
void takesA(covariant B b) {
print("Hello B");
}
}
void main() {
A a = new A();
A b = new B();
b.takesA(b);
b.takesA(a);
}
```
Before fix:
```
$ out/ReleaseX64/dart t.dart
Hello B
Unhandled exception:
type 'A' is not a subtype of type 'B' of 'b'
#0 B.takesA (file:///path/to/t.dart)
#1 main (file:///path/to/t.dart:16:5)
[...]
```
With fix:
```
$ out/ReleaseX64/dart t.dart
Hello B
Unhandled exception:
type 'A' is not a subtype of type 'B' of 'b'
#0 B.takesA (file:///path/to/t.dart:7:27)
#1 main (file:///path/to/t.dart:16:5)
[...]
```
TEST=runtime/tests/vm/dart/checked_parameter_assert_assignable_stacktrace_test.dart
Change-Id: I6409f6d9c41e9825391957f0df2b92b48ed0eb7a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302203
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Use JS `setTimeout` for events and `queueMicrotask` for micro-tasks.
dart2js event loop implementation is copied in `run_wasm.js` to be able
to use `clearTimeout`, `setInterval`, `clearInterval`, and
`scheduleMicrotask`, which are not not available in d8, and `setTimeout`
in d8 does not wait before calling a callback.
New passing d8 tests:
- co19/LibTest/async/Future/Future.delayed_A01_t02
- co19/LibTest/async/Stream/Stream.periodic_A01_t01
- co19/LibTest/async/Stream/Stream.periodic_all_t01
- co19/LibTest/async/Stream/Stream.periodic_all_t02
- co19/LibTest/async/Stream/timeout_A04_t01
- co19/LibTest/async/StreamController/stream_all_A01_t01
- co19/LibTest/async/StreamController/stream_all_A01_t02
- co19/LibTest/async/StreamController/stream_all_A02_t01
- co19/LibTest/async/StreamController/stream_all_A02_t02
- co19/LibTest/async/StreamController/StreamController.broadcast_Stream_all_A01_t01
- co19/LibTest/async/StreamController/StreamController.broadcast_Stream_all_A01_t02
- co19/LibTest/async/StreamController/StreamController.broadcast_Stream_all_A02_t01
- co19/LibTest/async/StreamController/StreamController.broadcast_Stream_all_A02_t02
- co19/LibTest/async/Timer/Timer.periodic_A01_t01
- co19/LibTest/async/Timer/Timer_A01_t01
- co19/LibTest/core/Stopwatch/elapsedTicks_A01_t01
- language/async/call_test
- language/regress/regress21795_test
- lib/async/multiple_timer_test
- lib/async/periodic_timer2_test
- lib/async/periodic_timer3_test
- lib/async/periodic_timer4_test
- lib/async/schedule_microtask3_test
- lib/async/schedule_microtask_test
- lib/async/stream_timeout_test
- lib/async/timer_isActive_test
- lib/async/timer_repeat_test
- lib/async/timer_test
New passing Chrome tests:
- co19/LibTest/async/Stream/timeout_A04_t01
- language/async/call_test
- lib/async/schedule_microtask3_test
Tests below fail because of async* desugaring issues and will be fixed
separately:
- language/async_star/no_cancel2_test
- language/async_star/no_cancel_test
Tests below fail because of an existing issue (#29615):
- co19/LibTest/async/StreamController/StreamController.broadcast_Stream_all_A01_t03
- co19/LibTest/async/StreamController/StreamController.broadcast_Stream_all_A02_t03
- co19/LibTest/async/StreamController/stream_all_A02_t03
Fixes#51599.
Change-Id: Ib313e99bf3b3cb3bebeddc9e47dc77425ef94481
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305201
Reviewed-by: Aske Simon Christensen <askesc@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Ömer Ağacan <omersa@google.com>
Pointer.fromFunction returns a native function pointer to a Dart
function, but has the restriction that the native code must invoke it
on the same thread as the isolate that created it. RawVoidCallbacks is
a new API that is similar to Pointer.fromFunction. It returns a native
function pointer that can be invoked on any thread, with the
restriction that the Dart function must return void. Under the hood we
forward the function args over a port to the target isolate.
We're not 100% settled on the name of the class, but the overall API
design won't change. I'll make sure to get the naming finalized before
submitting this CL. Doc with discussion of naming:
https://docs.google.com/document/d/1z9Rgahoid2AhC9JXwsDAEODvlJS6dvBHCbcGkOxv_ws/edit?resourcekey=0-TbdNiSL-fdwskla02QaPwg#heading=h.te70ikwelbw8
Bug: https://github.com/dart-lang/sdk/issues/37022
Change-Id: Iba98f6f803c52919b942fa054df1060991574c8c
TEST=ffi_async_callback_test.dart
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/308860
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Marya Belanger <mbelanger@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Danny gave a good idea that checking for availability of a refactoring
should be cheap. Change method signature refactoring mostly satisfied
this, with one exception - we cannot compute formal parameters for
an ExecutableElement, because we need resolved AST to do this, and
the invoked method can be declared in a different file. We cannot
afford resolving other files while checking.
So, this CL separates availability checking, and preparing formal
parameters, postponing expensive opertions until the time when the
action is invoked.
Change-Id: Ic703d70717e41de304c9dbcef66cadb22c139ad8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310041
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This change updates the flow analysis support for field promotion
(which is not yet switched on by default) so that it supports field
accesses inside cascade expressions. The key moving parts are:
- The type hierarchy `PropertyTarget` (which is used by the client to
tell flow analysis whether the target of a property access is
`this`, `super`, or an ordinary expression) now has a new class,
`CascadePropertyTarget`, to represent the situation where the target
of the property access is an implicit reference to the target of the
innermost enclosing cascade expression.
- Flow analysis has two new methods on its API:
`cascadeExpression_afterTarget` and `cascadeExpression_end`, so that
the client can inform flow analysis when a cascade expression is
being analyzed.
- Flow analysis uses its `_makeTemporaryReference` method to track the
implicit temporary variable that stores the target of cascade
expressions. (This method was developed as part of flow analysis
support for patterns, where it creates the data structures necessary
to track the implicit variables that are created as part of pattern
desugaring).
- The "mini-AST" pseudo-language used by flow analysis unit tests now
has a way to represent cascade expressions and method invocations.
- In addition to unit tests for `_fe_analyzer_shared`, `analyzer`, and
`front_end`, there are new language tests in
`tests/language/inference_update_2` to test cascaded field
promotions in end-to-end fashion.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I21353bbc884ed599cb1739cecfb68ad1d975d18b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309220
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Type rules for classes appearing in the type hierarchy above types
in the current module were missing. Specifically types that appear
in instantiations of supertypes.
Issue: https://github.com/dart-lang/sdk/issues/48585
Change-Id: I11f2af1435f18ab7567766c865d9898d60b9272b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309827
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
When mixin applications introduce forwarding stubs with covariant
checks they can contain uses of type parameters that belong to the
synthetic mixin application class. The type environment needs to
be updated to contain those type parameters.
Also cleanup some comments, variable names, and the code to avoid
an assertion failure. Methods can contain covariant checks on the
arguments, not just setters.
Issue: https://github.com/dart-lang/sdk/issues/52688
Change-Id: Ifd91f76de85cb2092b0a3a0c4a808f2660eb5c17
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/308460
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
The type arguments have to be explicitly given for collection types,
but this should be doable given they are all constants and only certain
types are allowed.
Change-Id: I2721f37d194d73de1df81302298101adeb87534c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309460
Commit-Queue: Jake Macdonald <jakemac@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
This adds assertions that no verification errors are found.
This enables the distinction between compile-time errors and
verification during trybot testing, leaving the latter as crashes
rather than (unexpected) compile-time errors, while retaining the
good diagnostics.
Change-Id: I097d875a75e8343c67cb96190b53320d4799663c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309662
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This ensures that abstract methods have sync markers even when they
are erroneously marked with a sync*, async or async* marker. This
ensures that these are verifiable.
Change-Id: I6d5bed9305129c47306ad9fd0e248683a6577800
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307804
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This reverts commit c7b288492a.
Reason for revert: breaks tests in G3, b/287581831
Original change's description:
> [vm] Mark Zone memory as unallocated/allocated/uninitialized for Address and Memory Sanitizer.
>
> TEST=ci
> Change-Id: Ia283d9aefec767e6ccc4f1c88abba73ce1c35e87
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212022
> Reviewed-by: Brian Quinlan <bquinlan@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>
Change-Id: I94ce2141e7b16c8e4be6641253e37dfa82698486
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309861
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ilya Yanok <yanok@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
This allows us to keep symbol names for classes/methods/fields - even if
obfuscation is enabled, but has no other effect on tree shaker / ...
For go/dart-ama
TEST=vm/dart/keep_name_pragma_test
Change-Id: I66c0fc32217d9180f821658bae463f2c1d7fb1af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309740
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Auto-Submit: Martin Kustermann <kustermann@google.com>
In a future CL, I plan to refactor some of the flow analysis API so
that the CFE can supply subexpressions to flow analysis before they
are lowered, rather than after. This should help reduce the risk of
bugs where lowering leads to type promotion being lost
(e.g. https://github.com/dart-lang/sdk/issues/52183).
The refactor I'm planning will be easier if there are no calls to the
flow analysis API within flow analysis itself, so to prepare for it,
this CL moves the body of `FlowAnalysisImpl.functionExpression_begin`
to a private method, and updates both `functionExpression_begin` and
`lateInitializer_begin` (which use the same underlying logic) to call
that private method.
Bug: https://github.com/dart-lang/sdk/issues/52189
Change-Id: I1185418225b8a402670e6eece6599c326fdc234a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309440
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Use `#if defined(TESTING)` for defining and using test-only flag
variables.
TTSTestCase had several flag fields where most of the flag fields should
not be set at the same time. Instead, use a separate `expected_result`
field that precisely states what type of result we expect from running
the test, and perform more precise checking of the expected state prior
to invocation and after invocation based on that.
Remove the old `TESTING_runtime_fail_on_existing_STC_entry` flag
variable and the abort it caused when set if an STC entry was found
when entering the runtime. Instead, replace it with two new flag
variables:
* `TESTING_runtime_entered_on_TTS_invocation`: whether the runtime
was entered for any reason during TTS invocation.
* `TESTING_found_hash_STC_entry`: whether we found an entry in a
hash-based STC, and so didn't perform runtime assignability checking.
Also modify most of the tests to check that either running the same
test case twice gives the same results, or in the case of
`kNewSTCEntry`, that a subsequent run with `kExistingSTCEntry` finds
the added entry.
TEST=vm/cc/TTS
Change-Id: Ibc3f49766494d0c61e2f78835637b9596b7f2db7
Cq-Include-Trybots: luci.dart.try:vm-linux-debug-x64-try,vm-linux-release-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309285
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
If again and again editing, say, a `pubspec.yaml` with proper timing
there is a "temporary leak" that repairs itself once the analyzer
finishes (some time after the editing stops).
What happens is that old contexts are saved in the
`declarationsTracker` but eventually cleared in the
`afterContextsCreated` call.
In a test on Windows (on Linux I'd currently run into
https://dartbug.com/52703) I opened the entire "pkg" folder and edited
a `pubspec.yaml` file every 5 seconds.
The analyzer went from using something along the lines of 700MB of heap
to using around 2.5 GB of heap after 25 edits and 17GB (!) of heap
shortly before stopping at 250 `pubspec.yaml` edits.
After the editing stopped (and clicking the GC button in observatory)
the heap usage went down to ~650MB heap used.
This fixes the problem by clearing the `declarationsTracker` on the
`afterContextsDestroyed` call too. In the same test it stays at around
300-700MB of heap.
Possibly related to:
https://github.com/dart-lang/sdk/issues/48788https://github.com/dart-lang/sdk/issues/52447
Change-Id: Ia38cc946a1f36fa8c5b6804f79cbc8dd96c21030
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309722
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
