From 6d83eaebe287f23bc3dcfa4443e626ad7f3de248 Mon Sep 17 00:00:00 2001 From: David Iglesias Date: Wed, 23 Sep 2020 21:47:02 -0700 Subject: [PATCH] [web] Add canUpdateAsMatch to PersistedPlatformView. (flutter/engine#21313) --- .../lib/web_ui/dev/driver_manager.dart | 4 - .../lib/web_ui/lib/src/engine/history.dart | 3 - .../lib/src/engine/html/platform_view.dart | 17 +++- .../web_ui/lib/src/engine/html/surface.dart | 12 +-- .../engine/surface/platform_view_test.dart | 88 +++++++++++++++++++ 5 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 engine/src/flutter/lib/web_ui/test/engine/surface/platform_view_test.dart diff --git a/engine/src/flutter/lib/web_ui/dev/driver_manager.dart b/engine/src/flutter/lib/web_ui/dev/driver_manager.dart index 595c965d20c..753eb70d1c9 100644 --- a/engine/src/flutter/lib/web_ui/dev/driver_manager.dart +++ b/engine/src/flutter/lib/web_ui/dev/driver_manager.dart @@ -5,7 +5,6 @@ // @dart = 2.6 import 'dart:io' as io; -import 'package:meta/meta.dart'; import 'package:path/path.dart' as pathlib; import 'package:web_driver_installer/chrome_driver_installer.dart'; import 'package:web_driver_installer/firefox_driver_installer.dart'; @@ -184,14 +183,12 @@ class SafariDriverManager extends DriverManager { /// tests. abstract class DriverManager { /// Installation directory for browser's driver. - @protected final io.Directory _browserDriverDir; /// This is the parent directory for all drivers. /// /// This directory is saved to [temporaryDirectories] and deleted before /// tests shutdown. - @protected final io.Directory _drivers; DriverManager(String browser) @@ -223,7 +220,6 @@ abstract class DriverManager { Future _verifyDriverForLUCI(); - @protected Future _startDriver(); static DriverManager chooseDriver(String browser) { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/history.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/history.dart index 75ee90ace28..59e1ba5fddf 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/history.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/history.dart @@ -88,16 +88,13 @@ abstract class BrowserHistory { /// /// Subclasses should send appropriate system messages to update the flutter /// applications accordingly. - @protected void onPopState(covariant html.PopStateEvent event); /// Sets up any prerequisites to use this browser history class. - @protected Future setup() => Future.value(); /// Restore any modifications to the html browser history during the lifetime /// of this class. - @protected Future tearDown() => Future.value(); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/html/platform_view.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/html/platform_view.dart index 87213f79417..df76f70602e 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/html/platform_view.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/html/platform_view.dart @@ -75,6 +75,16 @@ class PersistedPlatformView extends PersistedLeafSurface { } } + // Platform Views can only be updated if their viewId matches. + @override + bool canUpdateAsMatch(PersistedSurface oldSurface) { + if (super.canUpdateAsMatch(oldSurface)) { + // super checks the runtimeType of the surface, so we can just cast... + return viewId == ((oldSurface as PersistedPlatformView).viewId); + } + return false; + } + @override double matchForUpdate(PersistedPlatformView existingSurface) { return existingSurface.viewId == viewId ? 0.0 : 1.0; @@ -82,11 +92,10 @@ class PersistedPlatformView extends PersistedLeafSurface { @override void update(PersistedPlatformView oldSurface) { + assert(viewId == oldSurface.viewId, 'PersistedPlatformView with different viewId should never be updated. Check the canUpdateAsMatch method.',); super.update(oldSurface); - if (viewId != oldSurface.viewId) { - // The content of the surface has to be rebuild if the viewId is changed. - build(); - } else if (dx != oldSurface.dx || + // Only update if the view has been resized + if (dx != oldSurface.dx || dy != oldSurface.dy || width != oldSurface.width || height != oldSurface.height) { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/html/surface.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/html/surface.dart index 6a06cae0d6f..68efb10a925 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/html/surface.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/html/surface.dart @@ -275,7 +275,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// retain came in. @mustCallSuper @visibleForTesting - @protected void revive() { assert(debugAssertSurfaceState(this, PersistedSurfaceState.released)); state = PersistedSurfaceState.created; @@ -304,6 +303,7 @@ abstract class PersistedSurface implements ui.EngineLayer { html.Element? rootElement; /// Whether this surface can update an existing [oldSurface]. + @mustCallSuper bool canUpdateAsMatch(PersistedSurface oldSurface) { return oldSurface.isActive && runtimeType == oldSurface.runtimeType; } @@ -342,7 +342,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// /// This is called when we failed to locate an existing DOM element to reuse, /// such as on the very first frame. - @protected @mustCallSuper void build() { if (rootElement != null) { @@ -371,7 +370,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// frame, we reuse old ones as much as possible. This method should only be /// called when [isTotalMatchFor] returns true for the [oldSurface]. Otherwise /// adopting the [oldSurface]'s elements could lead to correctness issues. - @protected @mustCallSuper void adoptElements(covariant PersistedSurface oldSurface) { assert(oldSurface.rootElement != null); @@ -399,7 +397,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// /// Attempts to reuse [oldSurface]'s DOM element, if possible. Otherwise, /// creates a new element by calling [build]. - @protected @mustCallSuper void update(covariant PersistedSurface oldSurface) { assert(oldSurface != null); // ignore: unnecessary_null_comparison @@ -424,7 +421,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// /// This is also different from [build], which constructs a brand new surface /// sub-tree. - @protected @mustCallSuper void retain() { assert(rootElement != null); @@ -448,7 +444,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// /// This method may be overridden by concrete implementations, for example, to /// recycle the resources owned by this surface. - @protected @mustCallSuper void discard() { assert(debugAssertSurfaceState(this, PersistedSurfaceState.active)); @@ -471,7 +466,6 @@ abstract class PersistedSurface implements ui.EngineLayer { state = PersistedSurfaceState.released; } - @protected @mustCallSuper void debugValidate(List validationErrors) { if (rootElement == null) { @@ -541,7 +535,6 @@ abstract class PersistedSurface implements ui.EngineLayer { /// clip behaviors. /// /// This method is called by the [preroll] method. - @protected void recomputeTransformAndClip() { _transform = parent!._transform; _localClipBounds = null; @@ -578,7 +571,6 @@ abstract class PersistedSurface implements ui.EngineLayer { } } - @protected @mustCallSuper void debugPrintAttributes(StringBuffer buffer) { if (rootElement != null) { @@ -586,7 +578,6 @@ abstract class PersistedSurface implements ui.EngineLayer { } } - @protected @mustCallSuper void debugPrintChildren(StringBuffer buffer, int indent) {} @@ -1155,7 +1146,6 @@ abstract class PersistedContainerSurface extends PersistedSurface { } @override - @protected @mustCallSuper void debugValidate(List validationErrors) { super.debugValidate(validationErrors); diff --git a/engine/src/flutter/lib/web_ui/test/engine/surface/platform_view_test.dart b/engine/src/flutter/lib/web_ui/test/engine/surface/platform_view_test.dart new file mode 100644 index 00000000000..eeda48709da --- /dev/null +++ b/engine/src/flutter/lib/web_ui/test/engine/surface/platform_view_test.dart @@ -0,0 +1,88 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @dart = 2.6 +import 'dart:async'; +import 'dart:html' as html; + +import 'package:ui/src/engine.dart'; +import 'package:ui/ui.dart'; + +import 'package:test/bootstrap/browser.dart'; +import 'package:test/test.dart'; + +import '../../matchers.dart'; + +const MethodCodec codec = StandardMethodCodec(); +final EngineWindow window = EngineWindow(); + +void main() { + internalBootstrapBrowserTest(() => testMain); +} + +void testMain() { + PersistedPlatformView view; + + group('PersistedPlatformView', () { + setUp(() async { + platformViewRegistry.registerViewFactory( + 'test-0', + (viewId) => html.DivElement(), + ); + platformViewRegistry.registerViewFactory( + 'test-1', + (viewId) => html.DivElement(), + ); + // Ensure the views are created... + await Future.wait([ + _createPlatformView(0, 'test-0'), + _createPlatformView(1, 'test-1'), + ]); + view = PersistedPlatformView(0, 0, 0, 100, 100)..build(); + }); + + group('update', () { + test('throws assertion error if called with different viewIds', () { + final differentView = PersistedPlatformView(1, 1, 1, 100, 100)..build(); + expect(() { + view.update(differentView); + }, throwsAssertionError); + }); + }); + + group('canUpdateAsMatch', () { + test('returns true when viewId is the same', () { + final sameView = PersistedPlatformView(0, 1, 1, 100, 100)..build(); + expect(view.canUpdateAsMatch(sameView), isTrue); + }); + + test('returns false when viewId is different', () { + final differentView = PersistedPlatformView(1, 1, 1, 100, 100)..build(); + expect(view.canUpdateAsMatch(differentView), isFalse); + }); + + test('returns false when other view is not a PlatformView', () { + final anyView = PersistedOpacity(null, 1, Offset(0, 0))..build(); + expect(view.canUpdateAsMatch(anyView), isFalse); + }); + }); + }); +} + +// Sends a platform message to create a Platform View with the given id and viewType. +Future _createPlatformView(int id, String viewType) { + final completer = Completer(); + window.sendPlatformMessage( + 'flutter/platform_views', + codec.encodeMethodCall(MethodCall( + 'create', + { + 'id': id, + 'viewType': viewType, + }, + )), + (dynamic _) => completer.complete(), + ); + return completer.future; +}