null-annotate remaining engine code #18852
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| return clampedX * clampedX + clampedY * clampedY; | ||
| } | ||
|
|
||
| class RawRecordingCanvas extends BitmapCanvas |
There was a problem hiding this comment.
This is not a new class. I just moved it from dart:ui to dart:_engine. It should have never been in dart:ui.
lib/ui/channel_buffers.dart
Outdated
|
|
||
| /// Returns true on overflow. | ||
| bool push(String/*!*/ channel, ByteData/*!*/ data, PlatformMessageResponseCallback/*!*/ callback) { | ||
| bool push(String/*!*/ channel, ByteData/*?*/ data, PlatformMessageResponseCallback/*!*/ callback) { |
There was a problem hiding this comment.
Changing this because looking at the code, the data simply flows through _StoredMessage, and it's up to the framework to interpret the data. Given that most other platform message API allow ByteData to be null, I'm making it nullable as well. However, handleMessage below is different. data cannot be null there because it's used by control channel only, which seems to have a more specific schema. In particular, it never passes null.
| /// recorded thus far. After calling this function, both the picture recorder | ||
| /// and the canvas objects are invalid and cannot be used further. | ||
| /// | ||
| /// Returns null if the PictureRecorder is not associated with a canvas. |
There was a problem hiding this comment.
Backpedaled on this change. Updating the dartdoc instead.
| /// This is equivalent to [pushTransform] with a matrix with only translation. | ||
| /// | ||
| /// See [pop] for details about the operation stack. | ||
| OffsetEngineLayer pushOffset( |
There was a problem hiding this comment.
Here and below, do these return types need annotations? Or do only the non-abstract subclasses need the annotations?
There was a problem hiding this comment.
These should be annotated. Abstract methods don't provide any signal to the tool to decide whether return type can or cannot be null. Done.
|
|
||
| /// Initializes domRenderer with specific devicePixelRatio and physicalSize. | ||
| Future<void> webOnlyInitializeTestDomRenderer({double devicePixelRatio = 3.0}) { | ||
| Future<void> webOnlyInitializeTestDomRenderer({double/*!*/ devicePixelRatio = 3.0}) { |
There was a problem hiding this comment.
| Future<void> webOnlyInitializeTestDomRenderer({double/*!*/ devicePixelRatio = 3.0}) { | |
| Future<void>/*!*/ webOnlyInitializeTestDomRenderer({double/*!*/ devicePixelRatio = 3.0}) { |
|
LUCI failures say "Infra failure", so will merge, but OK with reverting if this causes problems. |
* null-annotate remaining engine code
Null-annotate/revise remaining engine code.
Related Issues
flutter/flutter#53661