Reland 2 (part 1): Enforce the rule of calling FlutterView.Render

shell/common/shell.cc

@@ -144,31 +144,36 @@ void PerformInitializationTasks(Settings& settings) {
 
 }  // namespace
 
-std::unique_ptr<Shell> Shell::Create(
-    const PlatformData& platform_data,
-    const TaskRunners& task_runners,
-    Settings settings,
-    const Shell::CreateCallback<PlatformView>& on_create_platform_view,
-    const Shell::CreateCallback<Rasterizer>& on_create_rasterizer,
-    bool is_gpu_disabled) {
-  // This must come first as it initializes tracing.
-  PerformInitializationTasks(settings);
-
-  TRACE_EVENT0("flutter", "Shell::Create");
-
+std::pair<DartVMRef, fml::RefPtr<const DartSnapshot>>
+Shell::InferVmInitDataFromSettings(Settings& settings) {
   // Always use the `vm_snapshot` and `isolate_snapshot` provided by the
   // settings to launch the VM.  If the VM is already running, the snapshot
   // arguments are ignored.
   auto vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings);
   auto isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings);
   auto vm = DartVMRef::Create(settings, vm_snapshot, isolate_snapshot);
-  FML_CHECK(vm) << "Must be able to initialize the VM.";
 
   // If the settings did not specify an `isolate_snapshot`, fall back to the
   // one the VM was launched with.
   if (!isolate_snapshot) {
     isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot();
   }
+  return {std::move(vm), isolate_snapshot};
+}
+
+std::unique_ptr<Shell> Shell::Create(
+    const PlatformData& platform_data,
+    const TaskRunners& task_runners,
+    Settings settings,
+    const Shell::CreateCallback<PlatformView>& on_create_platform_view,
+    const Shell::CreateCallback<Rasterizer>& on_create_rasterizer,
+    bool is_gpu_disabled) {
+  // This must come first as it initializes tracing.
+  PerformInitializationTasks(settings);
+
+  TRACE_EVENT0("flutter", "Shell::Create");
+
+  auto [vm, isolate_snapshot] = InferVmInitDataFromSettings(settings);
   auto resource_cache_limit_calculator =
       std::make_shared<ResourceCacheLimitCalculator>(
           settings.resource_cache_max_bytes_threshold);

shell/common/shell.h

@@ -438,15 +438,29 @@ class Shell final : public PlatformView::Delegate,
   const std::shared_ptr<fml::ConcurrentTaskRunner>
   GetConcurrentWorkerTaskRunner() const;
 
+  // Infer the VM ref and the isolate snapshot based on the settings.
+  //
+  // If the VM is already running, the settings are ignored, but the returned
+  // isolate snapshot always prioritize what is specified by the settings, and
+  // falls back to the one VM was launched with.
+  //
+  // This function is what Shell::Create uses to infer snapshot settings.
+  //
+  // TODO(dkwingsmt): Extracting this method is part of a bigger change. If the
+  // entire change is not eventually landed, we should merge this method back
+  // to Create. https://github.com/flutter/flutter/issues/136826
+  static std::pair<DartVMRef, fml::RefPtr<const DartSnapshot>>
+  InferVmInitDataFromSettings(Settings& settings);
+
  private:
   using ServiceProtocolHandler =
       std::function<bool(const ServiceProtocol::Handler::ServiceProtocolMap&,
                          rapidjson::Document*)>;
 
   /// A collection of message channels (by name) that have sent at least one
-  /// message from a non-platform thread. Used to prevent printing the error log
-  /// more than once per channel, as a badly behaving plugin may send multiple
-  /// messages per second indefinitely.
+  /// message from a non-platform thread. Used to prevent printing the error
+  /// log more than once per channel, as a badly behaving plugin may send
+  /// multiple messages per second indefinitely.
   std::mutex misbehaving_message_channels_mutex_;
   std::set<std::string> misbehaving_message_channels_;
   const TaskRunners task_runners_;
@@ -497,19 +511,20 @@ class Shell final : public PlatformView::Delegate,
   bool frame_timings_report_scheduled_ = false;
 
   // Vector of FrameTiming::kCount * n timestamps for n frames whose timings
-  // have not been reported yet. Vector of ints instead of FrameTiming is stored
-  // here for easier conversions to Dart objects.
+  // have not been reported yet. Vector of ints instead of FrameTiming is
+  // stored here for easier conversions to Dart objects.
   std::vector<int64_t> unreported_timings_;
 
-  /// Manages the displays. This class is thread safe, can be accessed from any
-  /// of the threads.
+  /// Manages the displays. This class is thread safe, can be accessed from
+  /// any of the threads.
   std::unique_ptr<DisplayManager> display_manager_;
 
   // protects expected_frame_size_ which is set on platform thread and read on
   // raster thread
   std::mutex resize_mutex_;
 
-  // used to discard wrong size layer tree produced during interactive resizing
+  // used to discard wrong size layer tree produced during interactive
+  // resizing
   std::unordered_map<int64_t, SkISize> expected_frame_sizes_;
 
   // Used to communicate the right frame bounds via service protocol.
@@ -746,7 +761,8 @@ class Shell final : public PlatformView::Delegate,
 
   // Service protocol handler
   //
-  // The returned SkSLs are base64 encoded. Decode before storing them to files.
+  // The returned SkSLs are base64 encoded. Decode before storing them to
+  // files.
   bool OnServiceProtocolGetSkSLs(
       const ServiceProtocol::Handler::ServiceProtocolMap& params,
       rapidjson::Document* response);
@@ -767,8 +783,8 @@ class Shell final : public PlatformView::Delegate,
 
   // Service protocol handler
   //
-  // Forces the FontCollection to reload the font manifest. Used to support hot
-  // reload for fonts.
+  // Forces the FontCollection to reload the font manifest. Used to support
+  // hot reload for fonts.
   bool OnServiceProtocolReloadAssetFonts(
       const ServiceProtocol::Handler::ServiceProtocolMap& params,
       rapidjson::Document* response);