Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a HtmlElementView for the debugger page #1255

Merged
merged 26 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ pubspec.lock
# such as the position of icons or the choice of a background image.
.DS_Store

# A generated file not intended to be checked in to version control
# Generated files not intended to be checked in to version control
flutter_export_environment.sh

.flutter-plugins
generated_plugin_registrant.dart
Podfile
Podfile.lock
97 changes: 97 additions & 0 deletions packages/devtools_app/lib/debugger_html_plugin.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Copyright 2019 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.

// This code imports dart:ui, but it uses API calls
// that are only available in the web implementation of dart:ui.
import 'dart:ui' as dart_ui_web;

import 'package:flutter_web_plugins/flutter_web_plugins.dart';

// TODO(https://github.com/flutter/devtools/issues/1258): switch to dart:html
// when turning down html_shim.
// This is web plugin code may only be compiled into the web app.
import 'package:html_shim/html.dart' as html;

import 'src/debugger/html_debugger_screen.dart';
import 'src/framework/html_framework.dart';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

import 'src/ui/html_elements.dart';

/// A web-only Flutter plugin to show the [HtmlDebuggerScreen].
class DebuggerHtmlPlugin {
DebuggerHtmlPlugin();

HtmlFramework _framework;
HtmlDebuggerScreen _screen;
html.Element _viewRoot;

/// Registers this plugin with Flutter.
///
/// The pubspec.yaml tells Flutter to look at this class for a plugin.
/// When it finds the class, it invokes this static method from
/// `generated_plugin_registrant.dart`.
static void registerWith(Registrar registrar) {
final instance = DebuggerHtmlPlugin();
// This call is only defined for the web implementation of dart:ui.
// The regular dart UI cannot resolve this method call.
// TODO(https://github.com/flutter/flutter/issues/43377): Remove 'ignore'
// after the APIs match between flutter and web.
// ignore:undefined_prefixed_name
dart_ui_web.platformViewRegistry.registerViewFactory(
'DebuggerFlutterPlugin',
instance.build,
);
}

/// Builds the html content of the debugger plugin.
///
/// [viewId] is used to distinguish between multiple instances of the same
/// view, such as video players. We can ignore it on DevTools.
html.Element build(int viewId) {
if (_viewRoot != null) {
return _viewRoot;
}
// Flutter loads this view inside of a shadow DOM.
// To get our existing CSS to work, we wrap the shadow page with the regular
//
// <html><head></head><body></body></html>.
_viewRoot = html.Element.tag('html');
html.HttpRequest.getString('debugger_screen.html').then(_updateViewRoot);
return _viewRoot;
}

/// Loads the [HtmlDebuggerScreen] after receiving the debugger screen
/// template html.
void _updateViewRoot(String response) {
_viewRoot.setInnerHtml(
response,
treeSanitizer: html.NodeTreeSanitizer.trusted,
);

// Tell the DevTools html code that the root to query for elements from
// is under this overridden root.
overrideDocumentRoot = _viewRoot;
_framework = HtmlFramework();
_screen = HtmlDebuggerScreen();
_framework.addScreen(_screen);

// Wait for the content to attach to the page, then load the debugger
// screen.
final observer = html.MutationObserver((mutations, observer) {
if (_framework.mainElement.element.isConnected) {
observer.disconnect();
_framework.load(_screen);
}
});
observer.observe(html.document, subtree: true, childList: true);

// TODO(https://github.com/flutter/flutter/issues/43520): This works around
// Flutter taking the mouse wheel events.
_viewRoot.onWheel.listen((event) {
event.stopImmediatePropagation();
});
_viewRoot.onMouseWheel.listen((event) {
event.stopImmediatePropagation();
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Copyright 2019 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.

import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter_icons/flutter_icons.dart';

import '../../flutter/screen.dart';

class DebuggerScreen extends Screen {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file turned out nice and simple.

const DebuggerScreen() : super('Debugger');

@override
Widget build(BuildContext context) {
return DebuggerScreenBody();
}

@override
Widget buildTab(BuildContext context) {
return Tab(
text: 'Debugger',
icon: Icon(Octicons.getIconData('bug')),
);
}
}

class DebuggerScreenBody extends StatelessWidget {
@override
Widget build(BuildContext context) {
if (!kIsWeb) {
final theme = Theme.of(context);
final textStyle = Theme.of(context)
.textTheme
.headline
.copyWith(color: theme.accentColor);
return Center(
child: Text(
'The debugger screen is only available when running DevTools as a web'
'app.\n'
'\n'
'It is implemented as a webview, which is not available in Flutter '
'desktop embedding.',
style: textStyle,
textAlign: TextAlign.center,
),
);
}

// TODO(https://github.com/flutter/flutter/issues/43532): Don't build const
// because compile time const evaluation will fail on non-web apps.
// ignore:prefer_const_constructors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
}

final webView = HtmlElementView(
viewType: 'DebuggerFlutterPlugin',
);

// Wrap the content with an EagerGestureRecognizer to pass all mouse
// events to the web view.
return RawGestureDetector(
gestures: {
EagerGestureRecognizer: _EagerGestureFactory(PointerDeviceKind.mouse),
},
child: webView,
);
}
}

class _EagerGestureFactory
extends GestureRecognizerFactory<EagerGestureRecognizer> {
_EagerGestureFactory(this.kind);

final PointerDeviceKind kind;

@override
EagerGestureRecognizer constructor() => EagerGestureRecognizer(kind: kind);

@override
void initializer(EagerGestureRecognizer instance) {}
}
4 changes: 3 additions & 1 deletion packages/devtools_app/lib/src/flutter/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

import 'dart:async';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';

import '../../src/framework/framework_core.dart';
import '../../src/globals.dart';
import '../debugger/flutter/debugger_screen.dart';
import '../info/flutter/info_screen.dart';
import '../inspector/flutter/inspector_screen.dart';
import '../performance/flutter/performance_screen.dart';
Expand Down Expand Up @@ -68,7 +70,7 @@ class DevToolsAppState extends State<DevToolsApp> {
EmptyScreen.timeline,
const PerformanceScreen(),
EmptyScreen.memory,
EmptyScreen.debugger,
const DebuggerScreen(),
EmptyScreen.logging,
const InfoScreen(),
],
Expand Down
5 changes: 0 additions & 5 deletions packages/devtools_app/lib/src/flutter/screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ abstract class Screen {
class EmptyScreen extends Screen {
const EmptyScreen(String name, this.icon) : super(name);

static final EmptyScreen debugger = EmptyScreen(
'Debugger',
Octicons.getIconData('bug'),
);

static final EmptyScreen timeline = EmptyScreen(
'Timeline',
Octicons.getIconData('pulse'),
Expand Down
14 changes: 12 additions & 2 deletions packages/devtools_app/lib/src/ui/html_elements.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,18 @@
import 'dart:async';
import 'package:html_shim/html.dart' hide Point;

/// Override of the default document root to allow DevTools to work
/// inside of a shadow DOM.
///
/// When this is not null, [querySelector] calls should use it
/// instead of [document.querySelector(selectors)] to retrieve
/// elements.
Element overrideDocumentRoot;

/// Finds the first descendant element of this document with the given id.
Element queryId(String id) => querySelector('#$id');
Element queryId(String id) => overrideDocumentRoot != null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. I wouldn't be very confident this is the only way we are querying by id or querySelector so definitely audit the debugger code to make sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also a call to querySelectorAll in framework, but it's for the main navbar, so it's not needed.

? overrideDocumentRoot.querySelector('#$id')
: querySelector('#$id');

CoreElement a({String text, String c, String a, String href, String target}) =>
CoreElement('a', text: text, classes: c, attributes: a)
Expand Down Expand Up @@ -97,7 +107,7 @@ class CoreElement {
}
}

CoreElement.from(this.element);
CoreElement.from(this.element) : assert(element != null);

final Element element;

Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
#include "Pods/Target Support Files/Pods-Runner/Pods-Runner.debug.xcconfig"
#include "ephemeral/Flutter-Generated.xcconfig"
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
#include "Pods/Target Support Files/Pods-Runner/Pods-Runner.release.xcconfig"
#include "ephemeral/Flutter-Generated.xcconfig"
Loading