diff --git a/packages/flutter/lib/src/foundation/assertions.dart b/packages/flutter/lib/src/foundation/assertions.dart index bf400ac13ec..0d88801e06f 100644 --- a/packages/flutter/lib/src/foundation/assertions.dart +++ b/packages/flutter/lib/src/foundation/assertions.dart @@ -59,6 +59,11 @@ class FlutterErrorDetails { /// A human-readable description of where the error was caught (as opposed to /// where it was thrown). + /// + /// The string should be in a form that will make sense in English when + /// following the word "thrown", as in "thrown while obtaining the image from + /// the network" (for the context "while obtaining the image from the + /// network"). final String context; /// A callback which filters the [stack] trace. Receives an iterable of diff --git a/packages/flutter/lib/src/material/input_decorator.dart b/packages/flutter/lib/src/material/input_decorator.dart index 772f15c265e..8fa4e1e67dd 100644 --- a/packages/flutter/lib/src/material/input_decorator.dart +++ b/packages/flutter/lib/src/material/input_decorator.dart @@ -2268,7 +2268,7 @@ class InputDecoration { /// can be expanded beyond that. Anything larger than 24px will require /// additional padding to ensure it matches the material spec of 12px padding /// between the right edge of the input and trailing edge of the prefix icon. - /// The following snipped shows how to pad the trailing edge of the suffix + /// The following snippet shows how to pad the trailing edge of the suffix /// icon: /// /// ```dart diff --git a/packages/flutter/lib/src/painting/image_stream.dart b/packages/flutter/lib/src/painting/image_stream.dart index 2a20a328f5b..335f1d083e0 100644 --- a/packages/flutter/lib/src/painting/image_stream.dart +++ b/packages/flutter/lib/src/painting/image_stream.dart @@ -61,7 +61,7 @@ class ImageInfo { /// Used by [ImageStream]. /// /// The `synchronousCall` argument is true if the listener is being invoked -/// during the call to addListener. This can be useful if, for example, +/// during the call to `addListener`. This can be useful if, for example, /// [ImageStream.addListener] is invoked during a frame, so that a new rendering /// frame is requested if the call was asynchronous (after the current frame) /// and no rendering frame is requested if the call was synchronous (within the @@ -148,9 +148,13 @@ class ImageStream extends Diagnosticable { /// `listener`. If an error occurred, `onError` will be called instead of /// `listener`. /// - /// Many `listener`s can have the same `onError` and one `listener` can also - /// have multiple `onError` by invoking [addListener] multiple times with - /// a different `onError` each time. + /// If a `listener` or `onError` handler is registered multiple times, then it + /// will be called multiple times when the image stream completes (whether + /// because a new image is available or because an error occurs, + /// respectively). In general, registering a listener multiple times is + /// discouraged because [removeListener] will remove the first instance that + /// was added, even if it was added with a different `onError` than the + /// intended paired `addListener` call. void addListener(ImageListener listener, { ImageErrorListener onError }) { if (_completer != null) return _completer.addListener(listener, onError: onError); @@ -160,11 +164,26 @@ class ImageStream extends Diagnosticable { /// Stop listening for new concrete [ImageInfo] objects and errors from /// the `listener`'s associated [ImageErrorListener]. + /// + /// If `listener` has been added multiple times, this removes the first + /// instance of the listener, along with the `onError` listener that was + /// registered with that first instance. This might not be the instance that + /// the `addListener` corresponding to this `removeListener` had added. + /// + /// For example, if one widget calls [addListener] with a global static + /// function and a private error handler, and another widget calls + /// [addListener] with the same global static function but a different private + /// error handler, then the second widget is disposed and removes the image + /// listener (the aforementioned global static function), it will remove the + /// error handler from the first widget, not the second. If an error later + /// occurs, the first widget, which is still supposedly listening, will not + /// receive any messages, while the second, which is supposedly disposed, will + /// have its callback invoked. void removeListener(ImageListener listener) { if (_completer != null) return _completer.removeListener(listener); assert(_listeners != null); - for (int i = 0; i < _listeners.length; ++i) { + for (int i = 0; i < _listeners.length; i += 1) { if (_listeners[i].listener == listener) { _listeners.removeAt(i); break; @@ -216,6 +235,24 @@ abstract class ImageStreamCompleter extends Diagnosticable { ImageInfo _currentImage; FlutterErrorDetails _currentError; + /// Whether any listeners are currently registered. + /// + /// Clients should not depend on this value for their behavior, because having + /// one listener's logic change when another listener happens to start or stop + /// listening will lead to extremely hard-to-track bugs. Subclasses might use + /// this information to determine whether to do any work when there are no + /// listeners, however; for example, [MultiFrameImageStreamCompleter] uses it + /// to determine when to iterate through frames of an animated image. + /// + /// Typically this is used by overriding [addListener], checking if + /// [hasListeners] is false before calling `super.addListener()`, and if so, + /// starting whatever work is needed to determine when to call + /// [notifyListeners]; and similarly, by overriding [removeListener], checking + /// if [hasListeners] is false after calling `super.removeListener()`, and if + /// so, stopping that same work. + @protected + bool get hasListeners => _listeners.isNotEmpty; + /// Adds a listener callback that is called whenever a new concrete [ImageInfo] /// object is available or an error is reported. If a concrete image is /// already available, or if an error has been already reported, this object @@ -228,6 +265,18 @@ abstract class ImageStreamCompleter extends Diagnosticable { /// occurred. If the listener is added within a render object paint function, /// then use this flag to avoid calling [RenderObject.markNeedsPaint] during /// a paint. + /// + /// An [ImageErrorListener] can also optionally be added along with the + /// `listener`. If an error occurred, `onError` will be called instead of + /// `listener`. + /// + /// If a `listener` or `onError` handler is registered multiple times, then it + /// will be called multiple times when the image stream completes (whether + /// because a new image is available or because an error occurs, + /// respectively). In general, registering a listener multiple times is + /// discouraged because [removeListener] will remove the first instance that + /// was added, even if it was added with a different `onError` than the + /// intended paired `addListener` call. void addListener(ImageListener listener, { ImageErrorListener onError }) { _listeners.add(_ImageListenerPair(listener, onError)); if (_currentImage != null) { @@ -259,8 +308,13 @@ abstract class ImageStreamCompleter extends Diagnosticable { /// Stop listening for new concrete [ImageInfo] objects and errors from /// its associated [ImageErrorListener]. + /// + /// If `listener` has been added multiple times, this removes the first + /// instance of the listener, along with the `onError` listener that was + /// registered with that first instance. This might not be the instance that + /// the `addListener` corresponding to this `removeListener` had added. void removeListener(ImageListener listener) { - for (int i = 0; i < _listeners.length; ++i) { + for (int i = 0; i < _listeners.length; i += 1) { if (_listeners[i].listener == listener) { _listeners.removeAt(i); break; @@ -295,6 +349,29 @@ abstract class ImageStreamCompleter extends Diagnosticable { /// /// If no error listeners are attached, a [FlutterError] will be reported /// instead. + /// + /// The `context` should be a string describing where the error was caught, in + /// a form that will make sense in English when following the word "thrown", + /// as in "thrown while obtaining the image from the network" (for the context + /// "while obtaining the image from the network"). + /// + /// The `exception` is the error being reported; the `stack` is the + /// [StackTrace] associated with the exception. + /// + /// The `informationCollector` is a callback (of type [InformationCollector]) + /// that is called when the exception is used by [FlutterError.reportError]. + /// It is used to obtain further details to include in the logs, which may be + /// expensive to collect, and thus should only be collected if the error is to + /// be logged in the first place. + /// + /// The `silent` argument causes the exception to not be reported to the logs + /// in release builds, if passed to [FlutterError.reportError]. (It is still + /// sent to error handlers.) It should be set to true if the error is one that + /// is expected to be encountered in release builds, for example network + /// errors. That way, logs on end-user devices will not have spurious + /// messages, but errors during development will still be reported. + /// + /// See [FlutterErrorDetails] for further details on these values. @protected void reportError({ String context, @@ -313,11 +390,11 @@ abstract class ImageStreamCompleter extends Diagnosticable { ); final List localErrorListeners = - _listeners.map( - (_ImageListenerPair listenerPair) => listenerPair.errorListener - ).where( - (ImageErrorListener errorListener) => errorListener != null - ).toList(); + _listeners.map( + (_ImageListenerPair listenerPair) => listenerPair.errorListener + ).where( + (ImageErrorListener errorListener) => errorListener != null + ).toList(); if (localErrorListeners.isEmpty) { FlutterError.reportError(_currentError); @@ -328,7 +405,7 @@ abstract class ImageStreamCompleter extends Diagnosticable { } catch (exception, stack) { FlutterError.reportError( FlutterErrorDetails( - context: 'by an image error listener', + context: 'when reporting an error to an image listener', library: 'image resource service', exception: exception, stack: stack, @@ -465,7 +542,7 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { } void _handleAppFrame(Duration timestamp) { - if (!_hasActiveListeners) + if (!hasListeners) return; if (_isFirstFrame() || _hasFrameDurationPassed(timestamp)) { _emitFrame(ImageInfo(image: _nextFrame.image, scale: _scale)); @@ -520,20 +597,17 @@ class MultiFrameImageStreamCompleter extends ImageStreamCompleter { _framesEmitted += 1; } - bool get _hasActiveListeners => _listeners.isNotEmpty; - @override void addListener(ImageListener listener, { ImageErrorListener onError }) { - if (!_hasActiveListeners && _codec != null) { + if (!hasListeners && _codec != null) _decodeNextFrameAndSchedule(); - } super.addListener(listener, onError: onError); } @override void removeListener(ImageListener listener) { super.removeListener(listener); - if (!_hasActiveListeners) { + if (!hasListeners) { _timer?.cancel(); _timer = null; } diff --git a/packages/flutter/test/foundation/caching_iterable_test.dart b/packages/flutter/test/foundation/caching_iterable_test.dart index 39e345eee05..fa6a10dc4bc 100644 --- a/packages/flutter/test/foundation/caching_iterable_test.dart +++ b/packages/flutter/test/foundation/caching_iterable_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:flutter/foundation.dart'; + import '../flutter_test_alternative.dart'; int yieldCount; diff --git a/packages/flutter/test/foundation/change_notifier_test.dart b/packages/flutter/test/foundation/change_notifier_test.dart index 6786fad5c93..0cbc0ffd3e0 100644 --- a/packages/flutter/test/foundation/change_notifier_test.dart +++ b/packages/flutter/test/foundation/change_notifier_test.dart @@ -1,4 +1,4 @@ - // Copyright 2016 The Chromium Authors. All rights reserved. +// Copyright 2016 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file.