Use a `HtmlElementView` for the debugger page #1255

DaveShuckerow · 2019-10-24T22:23:25Z

Caveats:

Scrollbars on the codemirror text view don't appear to always work.
Mouse scroll events aren't getting passed to the html view without overriding those events ourselves (Using an EagerGestureDetector on web doesn't appear to properly pass scroll wheel events flutter#43520).

For cross-referencing, this is the list of issues I've generated in Flutter out of this PR:

flutter test --platform chrome not generating generated_plugin_registrants.dart flutter#43538
const HtmlElementView prevents me from compiling non-web platforms flutter#43532
Using an EagerGestureDetector on web doesn't appear to properly pass scroll wheel events flutter#43520
Code generator is handling slashes improperly in directory names: flutter#43382
HtmlElementView API docs don't explain how to register an html platform view on the browser side flutter#43377

DaveShuckerow · 2019-10-24T23:02:30Z

I needed to override how we querySelectors for the debug page to run properly. I've cleaned up the way I do that and explained how it's used.

Please review and ask questions.

DaveShuckerow · 2019-10-24T23:19:51Z

I am currently seeing this exception when loading the plugin I've created.

══╡ EXCEPTION CAUGHT BY SERVICES LIBRARY ╞══════════════════════════════════════════════════════════
The following StateError was thrown during a platform message response callback:
Bad state: Future already completed

When the exception was thrown, this was the stack:
dart:sdk_internal 37842:62                                  complete
package:flutter/src/services/binding.dart 177:18            <fn>
dart:sdk_internal 134370:31                                 push
package:flutter_web_plugins/src/plugin_registry.dart 84:27  handlePlatformMessage
dart:sdk_internal 42389:7                                   _async
package:flutter_web_plugins/src/plugin_registry.dart 76:37  handlePlatformMessage
dart:sdk_internal 159840:17                                 sendPlatformMessage
package:flutter/src/services/binding.dart 175:15            [_sendPlatformMessage]
package:flutter/src/services/binding.dart 221:12            send
package:flutter/src/services/platform_channel.dart 314:51   invokeMethod
dart:sdk_internal 42389:7                                   _async
package:flutter/src/services/platform_channel.dart 312:28   invokeMethod
package:flutter/src/services/platform_channel.dart 446:3    [invokeMethod]
package:flutter/src/services/platform_channel.dart 429:36   invokeMethod$
dart:sdk_internal 42389:7                                   _async
package:flutter/src/services/platform_channel.dart 427:28   invokeMethod
package:flutter/src/services/system_sound.dart 23:35        play
dart:sdk_internal 42389:7                                   _async
package:flutter/src/services/system_sound.dart 22:27        play
package:flutter/src/material/feedback.dart 98:28            forTap
dart:sdk_internal 42389:7                                   _async
package:flutter/src/material/feedback.dart 93:29            forTap
package:flutter/src/material/ink_well.dart 700:33           [_handleTap]
package:flutter/src/material/ink_well.dart 783:36           <fn>
package:flutter/src/gestures/recognizer.dart 182:24         invokeCallback
package:flutter/src/gestures/tap.dart 486:47                handleTapUp
package:flutter/src/gestures/tap.dart 264:5                 [_checkUp]
package:flutter/src/gestures/tap.dart 199:7                 handlePrimaryPointer
package:flutter/src/gestures/recognizer.dart 467:9          handleEvent
package:flutter/src/gestures/pointer_router.dart 75:12      [_dispatch]
package:flutter/src/gestures/pointer_router.dart 102:34     route
package:flutter/src/gestures/binding.dart 218:19            handleEvent
package:flutter/src/gestures/binding.dart 198:14            dispatchEvent
package:flutter/src/gestures/binding.dart 156:7             [_handlePointerEvent]
package:flutter/src/gestures/binding.dart 102:7             [_flushPointerEventQueue]
package:flutter/src/gestures/binding.dart 86:32             [_handlePointerDataPacket]
dart:sdk_internal 145346:11                                 <fn>
════════════════════════════════════════════════════════════════════════════════════════════════════
Another exception was thrown: Bad state: Future already completed

jacob314

Can we make a Framework interface implemented by the dart:html and the Flutter versions of the app?
It should be pretty easy to make the couple of use cases covered by the debugger page work with the interface and then this page of the inspector can display toast and warning messages in a consistent way to the rest of the flutter things instead of doing odd and probably broken things when it needs to display warning messages, etc.

jacob314 · 2019-10-24T23:15:00Z

packages/devtools_app/lib/debugger_html_plugin.dart

+
+// We actually use dart:html here, no shim.
+// This code may only be compiled into the web app.
+import 'dart:ui' as web_ui;


dart:ui should be aliased as ui or dart_ui not web_ui.

My goal here is to be very specific: Right now this code will only work in the Flutter web implementation of dart:ui.

Happy to consider alternatives, but it's important to get that across to readers of this code.

how about
dart_ui_web
web_ui made me think dart:html but it might be just me.

jacob314 · 2019-10-24T23:18:29Z

packages/devtools_app/lib/debugger_html_plugin.dart

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// We actually use dart:html here, no shim.


remove the
// we actually use dart:html here, no shim.
All uses of html_shim are really using dart:html and throwing exceptions on other platforms.

My goal here is to point out that this code and its transitive dependencies are intended to only run as web code, in spite of being part of the flutter app.

When we turn down the html app, we want to switch from html shim to regular dart:html.

jacob314 · 2019-10-24T23:19:11Z

packages/devtools_app/lib/debugger_html_plugin.dart

+import 'package:html_shim/html.dart' as html;
+
+import 'src/debugger/html_debugger_screen.dart';
+import 'src/framework/html_framework.dart';


including all of html_framework.dart worries me a bit as there is going to be a bunch of intentional overlap with the flutter framework.

We need to quantify exactly which functionality needs to get tied in with Flutter. We can discuss this in detail in #1259

jacob314 · 2019-10-24T23:20:56Z

packages/devtools_app/lib/debugger_html_plugin.dart

+      return _viewRoot;
+    }
+    _viewRoot = html.Element.tag('html');
+    html.HttpRequest.getString('debugger_screen.html').then((response) {


add an async helper method so you can use async await rather than then here.

A then is necessary here. This method is getting passed as a PlatformViewFactory, which is synchronous.

jacob314 · 2019-10-24T23:22:41Z

packages/devtools_app/lib/debugger_html_plugin.dart

+      print('Built framework, building screen.');
+      _screen = HtmlDebuggerScreen();
+      _framework.addScreen(_screen);
+      final observer = html.MutationObserver((mutations, observer) {


why are we using mutation observers here? Add a comment. It isn't clear whether you are hacking around something in the html framework or if this is something to do with Flutter plugins.

This is waiting for content to attach to the page. I've updated the comments.

jacob314 · 2019-10-24T23:32:34Z

packages/devtools_app/web/debugger_screen.html

+    <link href="packages/octicons_css/octicons_4.4.0.css" rel="stylesheet">
+    <link href="packages/polymer_css/flex.css" rel="stylesheet">
+    <link href="packages/devtools_app/src/debugger/debugger.css" rel="stylesheet">
+    <link href="packages/devtools_app/src/inspector/inspector.css" rel="stylesheet">


remove the inspector, logging, performance, profiler, info, and timeline
css files.

jacob314 · 2019-10-24T23:33:02Z

packages/devtools_app/web/debugger_screen.html

+    <link href="packages/codemirror/codemirror.css" rel="stylesheet">
+    <script src="packages/codemirror/codemirror.js"></script>
+
+    <script src="packages/devtools_app/src/ui/plotly.js"></script>


remove plotly.

jacob314 · 2019-10-24T23:33:10Z

packages/devtools_app/web/debugger_screen.html

+        }
+    </style>
+
+    <script src="packages/ansi_up/ansi_up.js"></script>


add TODO to remove ansi_up.js

What is ansi_up used for in the debugger view?

A flutter-specific implementation of ansi will probably end up different from the web implementation and we may not be able to use it.

I believe ansi_up is used for debugger console output

jacob314 · 2019-10-24T23:33:43Z

packages/devtools_app/web/debugger_screen.html

+        <nav class="masthead-nav" id="main-nav">
+            <!-- add the known pages -->
+            <a disabled>
+                <span class="octicon octicon-device-mobile"></span>


why is all this content still here? it doesn't make sense if this is just the debugger screen.

The framework crashes if it can't find this content. I've marked it as hidden to get it to work.

Ideally, we strip down Framework and only take the steps necessary to show the debugger screen.

better yet. rewrite the debugger page so the only dart:html dependency is the codemirror portion rather than the full page :)

jacob314 · 2019-10-24T23:34:06Z

packages/devtools_app/web/debugger_screen.html

+    <div id="messages-container" class="container legacy-dart"></div>
+
+    <form id="ga-dialog" class="container legacy-dart"></form>
+    <form id="connect-dialog" class="container legacy-dart"></form>


remove this as well. Basically strip this file way down to just what is needed for the debugger page.

See previous comment.

kenzieschmoll · 2019-10-25T16:16:43Z

packages/devtools_app/lib/debugger_html_plugin.dart

+    final instance = DebuggerHtmlPlugin();
+    // ignore:undefined_prefixed_name
+    web_ui.platformViewRegistry
+        .registerViewFactory('DebuggerFlutterPlugin', instance.build);


nit: trailing comma

kenzieschmoll · 2019-10-25T16:23:07Z

packages/devtools_app/web/debugger_screen.html

+<!DOCTYPE html>
+
+<!--
+  Copyright 2018 The Chromium Authors. All rights reserved.


Good catch.

kenzieschmoll · 2019-10-25T16:25:39Z

packages/devtools_app/web/debugger_screen.html

+
+    <form id="ga-dialog" class="container legacy-dart"></form>
+    <form id="connect-dialog" class="container legacy-dart"></form>
+    <form id="snapshot-message" class="container legacy-dart"></form>


remove snapshot message

See my discussion about the html framework above.

kenzieschmoll · 2019-10-25T16:26:40Z

packages/devtools_app/web/debugger_screen.html

@@ -0,0 +1,152 @@
+<!DOCTYPE html>


For the code shared between this page and index.html, maybe add a comment to both that updating shared code in one file requires updating the other.

Good idea, done.

jacob314 · 2019-10-25T20:16:51Z

packages/devtools_app/lib/src/debugger/flutter/debugger_screen.dart

+          .copyWith(color: theme.accentColor);
+      return Center(
+        child: Text(
+          'The debugger screen is only available on web.',


Add a second line explaining why to avoid any confusion that we thought we were debugging a non web app.
For example:
"Because we have yet to port the debugger to Flutter"

jacob314 · 2019-10-25T20:18:25Z

packages/devtools_app/lib/src/debugger/flutter/debugger_screen.dart

    // Don't build this in const because compile time const evaluation
    // will fail on non-web apps.
+    // TODO(djshuckerow): Follow up on this: if this code won't compile
+    // on desktop as a const, then the constructor shouldn't be const.
+    // ignore:prefer_const_constructors


Use conditional imports instead of hacking around the const limitations.

On the web you would have this Screen widget.
On regular flutter you would have the kIsWeb fallback empty widget.
You can trigger which import is executed based on whether dart:html is available.

We should treat this as a bug in Flutter. Android and iOS native view widgets do not have this issue when building.

I want to make sure that future Flutter for web users don't need to be a special case.
flutter/flutter#43532

For showing platform views, we should be able to get a pattern like

switch(platform) { case Platform.android: return AndroidView(name); case Platform.web: return HtmlElementView(name); case Platform.macos: return MacosView(name); // etc. }

jacob314 · 2019-10-25T20:20:32Z

packages/devtools_app/web/debugger_screen.html

@@ -39,7 +39,9 @@
 </head>

 <body layout vertical style="width: 100%; height: 100%; background-color: transparent;">


There might be quite a flash of oddly styled content when this screen is displayed. As we can't remove the content until 1259 is landed in a separate cl, at least make all the unstyled content be hidden by default.

Everything except for the content div is already set to hidden.

The styles will be loaded before any of the other html content of this page, so I'm not sure what would come up as oddly styled.

I can set the entire body to be hidden until the framework loads the debugger screen, but I don't notice any difference between that and making no change.

that is all I was looking for

jacob314

lgtm with some nits.

…nd index

jacob314 · 2019-10-25T21:19:00Z

DaveShuckerow · 2019-10-29T22:09:17Z

why is always enabling web causing a problem?

If the generated_plugin_constants gets generated before package package:tuneup checks the code, the inspection fails.

We need to be able to tell tuneup or the analyzer to ignore lints in generated files.

DaveShuckerow · 2019-10-29T22:09:49Z

The only failure in the tests is caused by #1267, which this change doesn't affect. I'm merging this.

Dave Shuckerow added 7 commits October 23, 2019 13:05

Introduce inicial debugger page

8111dd0

Write a hello_world debugger screen

a1d4fb5

Try loading special content for the debugger screen

a793c20

Get the app showing

81bfe53

Make the content render properly

f185ab4

Handle sizing and background color from flutter app

e15cec0

Explain the root override

81f9856

DaveShuckerow marked this pull request as ready for review October 24, 2019 23:02

Dave Shuckerow added 2 commits October 24, 2019 16:18

Remove print statements

ba928e5

Organize imports

a396246

Dave Shuckerow added 2 commits October 24, 2019 16:25

Update to macos runners to make it work

6da8d83

Dont use const html view because it breaks cross-platform compilation

ef30194

jacob314 reviewed Oct 24, 2019

View reviewed changes

kenzieschmoll reviewed Oct 25, 2019

View reviewed changes

Dave Shuckerow added 2 commits October 25, 2019 10:04

Update comments

cebe84d

Get mouse events propagating properly

dfd3006

DaveShuckerow mentioned this pull request Oct 25, 2019

Using an EagerGestureDetector on web doesn't appear to properly pass scroll wheel events flutter/flutter#43520

Open

Dave Shuckerow added 4 commits October 25, 2019 11:30

Update comments

0173643

Show the debugger tab on desktop

2e5d812

More cleanup

2e33040

Travis needs the generated plugin registrant to run tests

9895b70

jacob314 reviewed Oct 25, 2019

View reviewed changes

jacob314 approved these changes Oct 25, 2019

View reviewed changes

Dave Shuckerow added 3 commits October 25, 2019 13:21

Add comments to html about the relationship between debugger_screen a…

3772e0f

…nd index

Update debugger availability comments

741bed9

Update debugger availability comments

8dec14d

Update TODOs

53f4fb0

Dave Shuckerow added 5 commits October 25, 2019 14:20

Rev version number of split

1013de8

Re-remove generated plugin registrant

98f50f3

Introduce workaround for flutter not generating plugin registrants

ae67ea7

enable-web in travis

a0614f5

Enable web only where needed

52b6efd

DaveShuckerow added the hummingbird Hummingbird implementation-related label Oct 29, 2019

DaveShuckerow merged commit 549b225 into flutter:master Oct 29, 2019

DaveShuckerow deleted the platform-view branch October 29, 2019 22:10

		@@ -39,7 +39,9 @@
		</head>

		<body layout vertical style="width: 100%; height: 100%; background-color: transparent;">

Use a HtmlElementView for the debugger page #1255

Use a HtmlElementView for the debugger page #1255

Conversation

DaveShuckerow commented Oct 24, 2019 • edited

DaveShuckerow commented Oct 24, 2019

DaveShuckerow commented Oct 24, 2019

jacob314 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacob314 left a comment

Choose a reason for hiding this comment

jacob314 commented Oct 25, 2019

DaveShuckerow commented Oct 29, 2019

DaveShuckerow commented Oct 29, 2019

Use a `HtmlElementView` for the debugger page #1255

Use a `HtmlElementView` for the debugger page #1255

DaveShuckerow commented Oct 24, 2019 •

edited