Review changes

Author: gspencergoog

lib/ui/compositing/scene.cc

@@ -45,7 +45,7 @@ Scene::Scene(std::shared_ptr<flutter::Layer> rootLayer,
   auto viewport_metrics = UIDartState::Current()
                               ->platform_configuration()
                               ->window()
-                              .viewport_metrics();
+                              ->viewport_metrics();
 
   layer_tree_ = std::make_unique<LayerTree>(
       SkISize::Make(viewport_metrics.physical_width,

lib/ui/painting/canvas.cc

@@ -429,7 +429,7 @@ void Canvas::drawShadow(const CanvasPath* path,
   SkScalar dpr = UIDartState::Current()
                      ->platform_configuration()
                      ->window()
-                     .viewport_metrics()
+                     ->viewport_metrics()
                      .device_pixel_ratio;
   external_allocation_size_ += path->path().approximateBytesUsed();
   flutter::PhysicalShapeLayer::DrawShadow(canvas_, path->path(), color,

lib/ui/window/platform_configuration.cc

@@ -188,18 +188,14 @@ PlatformConfigurationClient::~PlatformConfigurationClient() {}
 
 PlatformConfiguration::PlatformConfiguration(
     PlatformConfigurationClient* client)
-    : client_(client), window_(new Window({1.0, 0.0, 0.0})) {}
+    : client_(client) {}
 
 PlatformConfiguration::~PlatformConfiguration() {}
 
 void PlatformConfiguration::DidCreateIsolate() {
   library_.Set(tonic::DartState::Current(),
                Dart_LookupLibrary(tonic::ToDart("dart:ui")));
-}
-
-void PlatformConfiguration::SetWindowMetrics(
-    const ViewportMetrics& window_metrics) {
-  window_->UpdateWindowMetrics(library_, window_metrics);
+  window_.reset(new Window({1.0, 0.0, 0.0}));
 }
 
 void PlatformConfiguration::UpdateLocales(
@@ -293,20 +289,6 @@ void PlatformConfiguration::DispatchPlatformMessage(
                               tonic::ToDart(response_id)}));
 }
 
-void PlatformConfiguration::DispatchPointerDataPacket(
-    const PointerDataPacket& packet) {
-  std::shared_ptr<tonic::DartState> dart_state = library_.dart_state().lock();
-  if (!dart_state)
-    return;
-  tonic::DartState::Scope scope(dart_state);
-
-  Dart_Handle data_handle = ToByteData(packet.data());
-  if (Dart_IsError(data_handle))
-    return;
-  tonic::LogIfError(tonic::DartInvokeField(
-      library_.value(), "_dispatchPointerDataPacket", {data_handle}));
-}
-
 void PlatformConfiguration::DispatchSemanticsAction(int32_t id,
                                                     SemanticsAction action,
                                                     std::vector<uint8_t> args) {

lib/ui/window/platform_configuration.h

@@ -53,20 +53,24 @@ TEST(PlatformConfigurationTest, PlatformConfigurationInitialization) {
   PlatformConfiguration configuration(&client);
 
   ASSERT_EQ(configuration.client(), &client);
-  ASSERT_EQ(configuration.window().viewport_metrics().device_pixel_ratio, 1.0);
-  ASSERT_EQ(configuration.window().viewport_metrics().physical_width, 0.0);
-  ASSERT_EQ(configuration.window().viewport_metrics().physical_height, 0.0);
+  ASSERT_EQ(configuration.window()->viewport_metrics().device_pixel_ratio, 1.0);
+  ASSERT_EQ(configuration.window()->viewport_metrics().physical_width, 0.0);
+  ASSERT_EQ(configuration.window()->viewport_metrics().physical_height, 0.0);
 }
 
-TEST(PlatformConfigurationTest, PlatformConfigurationWindowMetricsUpdate) {
-  DummyPlatformConfigurationClient client;
-  PlatformConfiguration configuration(&client);
+// This doesn't actually work until I figure out how to instantiate an Isolate
+// and set it up properly.
 
-  configuration.SetWindowMetrics({2.0, 10.0, 20.0});
-  ASSERT_EQ(configuration.window().viewport_metrics().device_pixel_ratio, 2.0);
-  ASSERT_EQ(configuration.window().viewport_metrics().physical_width, 10.0);
-  ASSERT_EQ(configuration.window().viewport_metrics().physical_height, 20.0);
-}
+// TEST(PlatformConfigurationTest, PlatformConfigurationWindowMetricsUpdate) {
+//   DummyPlatformConfigurationClient client;
+//   PlatformConfiguration configuration(&client);
+//   configuration.DidCreateIsolate();
+//
+//   configuration.window()->UpdateWindowMetrics(ViewportMetrics{2.0, 10.0, 20.0});
+//   ASSERT_EQ(configuration.window()->viewport_metrics().device_pixel_ratio, 2.0);
+//   ASSERT_EQ(configuration.window()->viewport_metrics().physical_width, 10.0);
+//   ASSERT_EQ(configuration.window()->viewport_metrics().physical_height, 20.0);
+// }
 
 }  // namespace testing
 }  // namespace flutter

lib/ui/window/platform_configuration_unittests.cc

@@ -4,28 +4,47 @@
 
 #include "flutter/lib/ui/window/window.h"
 
-#include "lib/ui/window/viewport_metrics.h"
 #include "third_party/tonic/converter/dart_converter.h"
 #include "third_party/tonic/dart_args.h"
 #include "third_party/tonic/logging/dart_invoke.h"
+#include "third_party/tonic/typed_data/dart_byte_data.h"
 
 namespace flutter {
 
-Window::Window(ViewportMetrics metrics) : viewport_metrics_(metrics) {}
+Window::Window(ViewportMetrics metrics) : viewport_metrics_(metrics) {
+  library_.Set(tonic::DartState::Current(),
+               Dart_LookupLibrary(tonic::ToDart("dart:ui")));
+}
 
 Window::~Window() {}
 
-void Window::UpdateWindowMetrics(const tonic::DartPersistentValue& library,
-                                 const ViewportMetrics& metrics) {
+void Window::DispatchPointerDataPacket(
+    const PointerDataPacket& packet) {
+  std::shared_ptr<tonic::DartState> dart_state = library_.dart_state().lock();
+  if (!dart_state) {
+    return;
+  }
+  tonic::DartState::Scope scope(dart_state);
+
+  const std::vector<uint8_t>& buffer = packet.data();
+  Dart_Handle data_handle = tonic::DartByteData::Create(buffer.data(), buffer.size());
+  if (Dart_IsError(data_handle)) {
+    return;
+  }
+  tonic::LogIfError(tonic::DartInvokeField(
+      library_.value(), "_dispatchPointerDataPacket", {data_handle}));
+}
+
+void Window::UpdateWindowMetrics(const ViewportMetrics& metrics) {
   viewport_metrics_ = metrics;
 
-  std::shared_ptr<tonic::DartState> dart_state = library.dart_state().lock();
+  std::shared_ptr<tonic::DartState> dart_state = library_.dart_state().lock();
   if (!dart_state) {
     return;
   }
   tonic::DartState::Scope scope(dart_state);
   tonic::LogIfError(tonic::DartInvokeField(
-      library.value(), "_updateWindowMetrics",
+      library_.value(), "_updateWindowMetrics",
       {
           tonic::ToDart(metrics.device_pixel_ratio),
           tonic::ToDart(metrics.physical_width),

lib/ui/window/window.cc

@@ -24,10 +24,11 @@ class Window final {
 
   const ViewportMetrics& viewport_metrics() const { return viewport_metrics_; }
 
-  void UpdateWindowMetrics(const tonic::DartPersistentValue& library,
-                           const ViewportMetrics& metrics);
+  void DispatchPointerDataPacket(const PointerDataPacket& packet);
+  void UpdateWindowMetrics(const ViewportMetrics& metrics);
 
  private:
+  tonic::DartPersistentValue library_;
   ViewportMetrics viewport_metrics_;
 };
 

lib/ui/window/window.h

@@ -145,7 +145,7 @@ bool RuntimeController::SetViewportMetrics(const ViewportMetrics& metrics) {
   platform_data_.viewport_metrics = metrics;
 
   if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
-    platform_configuration->SetWindowMetrics(metrics);
+    platform_configuration->window()->UpdateWindowMetrics(metrics);
     return true;
   }
 
@@ -264,7 +264,7 @@ bool RuntimeController::DispatchPointerDataPacket(
   if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) {
     TRACE_EVENT1("flutter", "RuntimeController::DispatchPointerDataPacket",
                  "mode", "basic");
-    platform_configuration->DispatchPointerDataPacket(packet);
+    platform_configuration->window()->DispatchPointerDataPacket(packet);
     return true;
   }
 
@@ -324,6 +324,7 @@ FontCollection& RuntimeController::GetFontCollection() {
   return client_.GetFontCollection();
 }
 
+
 // |PlatformConfigurationClient|
 void RuntimeController::UpdateIsolateDescription(const std::string isolate_name,
                                                  int64_t isolate_port) {

runtime/runtime_controller.cc

@@ -421,7 +421,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
   ///             one frame interval from this point, the Flutter application
   ///             will jank.
   ///
-  ///             If an root isolate is running, this method calls the
+  ///             If a root isolate is running, this method calls the
   ///             `::_beginFrame` method in `hooks.dart`. If a root isolate is
   ///             not running, this call does nothing.
   ///
@@ -701,7 +701,7 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate {
 
   //----------------------------------------------------------------------------
   /// @brief      Notifies the engine that the embedder has expressed an opinion
-  ///             about where the accessibility tree should be generated or not.
+  ///             about whether the accessibility tree should be generated or not.
   ///             This call originates in the platform view and is forwarded to
   ///             the engine here on the UI task runner by the shell.
   ///

shell/common/engine.h

@@ -23,7 +23,7 @@ class PointerDataDispatcher;
 /// delivered packets, and dispatches them in sync with the VSYNC signal.
 ///
 /// This object will be owned by the engine because it relies on the engine's
-/// `Animator` (which owns `VsyncWaiter`) and `RuntomeController` to do the
+/// `Animator` (which owns `VsyncWaiter`) and `RuntimeController` to do the
 /// filtering. This object is currently designed to be only called from the UI
 /// thread (no thread safety is guaranteed).
 ///