chore: update to use new CreateGroupIsolate API

.github/workflows/shorebird_ci.yml

@@ -21,11 +21,6 @@ jobs:
 
     name: 🐦 Shorebird Test
 
-    # TODO(eseidel): This is also set inside shorebird_tests, unclear if
-    # if it's needed here as well.
-    env:
-      FLUTTER_STORAGE_BASE_URL: https://download.shorebird.dev
-
     steps:
       - name: 📚 Git Checkout
         uses: actions/checkout@v4

DEPS

@@ -15,7 +15,7 @@ vars = {
   'skia_git': 'https://skia.googlesource.com',
   'llvm_git': 'https://llvm.googlesource.com',
   'skia_revision': 'ea7cdbc6b986bbefcbac92fa429782e59518510f',
-  "dart_sdk_revision": "790993a4fb8ecf784de4e29532d75b027658f27e",
+  "dart_sdk_revision": "6a78a2deaee05bc74775fcfa2ff27aa53c96efca",
   "dart_sdk_git": "[email protected]:shorebirdtech/dart-sdk.git",
   "updater_git": "https://github.com/shorebirdtech/updater.git",
   "updater_rev": "76f005940db57c38b479cee858abc0cfbd12ac28",

engine/src/flutter/runtime/dart_isolate.cc

@@ -21,6 +21,10 @@
 #include "flutter/runtime/isolate_configuration.h"
 #include "flutter/runtime/platform_isolate_manager.h"
 #include "fml/message_loop_task_queues.h"
+
+#if SHOREBIRD_USE_INTERPRETER
+#include "flutter/shell/common/shorebird/shorebird.h"  // nogncheck
+#endif
 #include "fml/task_source.h"
 #include "fml/time/time_point.h"
 #include "third_party/dart/runtime/include/bin/native_assets_api.h"
@@ -245,6 +249,12 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
   } else {
     // The child isolate preparer is null but will be set when the isolate is
     // being prepared to run.
+#if SHOREBIRD_USE_INTERPRETER
+    // Get the base snapshot for Shorebird linking support (may be null).
+    fml::RefPtr<const DartSnapshot> base_snapshot = GetBaseIsolateSnapshot();
+#else
+    fml::RefPtr<const DartSnapshot> base_snapshot = nullptr;
+#endif
     isolate_group_data =
         std::make_unique<std::shared_ptr<DartIsolateGroupData>>(
             std::shared_ptr<DartIsolateGroupData>(new DartIsolateGroupData(
@@ -254,13 +264,30 @@ std::weak_ptr<DartIsolate> DartIsolate::CreateRootIsolate(
                 context.advisory_script_entrypoint,  // advisory entrypoint
                 nullptr,                             // child isolate preparer
                 isolate_create_callback,             // isolate create callback
-                isolate_shutdown_callback,        // isolate shutdown callback
-                std::move(native_assets_manager)  //
+                isolate_shutdown_callback,         // isolate shutdown callback
+                std::move(native_assets_manager),  // native assets manager
+                std::move(base_snapshot)           // base snapshot (Shorebird)
                 )));
     isolate_maker = [](std::shared_ptr<DartIsolateGroupData>*
                            isolate_group_data,
                        std::shared_ptr<DartIsolate>* isolate_data,
                        Dart_IsolateFlags* flags, char** error) {
+#if SHOREBIRD_USE_INTERPRETER
+      auto base_snapshot = (*isolate_group_data)->GetBaseSnapshot();
+      if (base_snapshot) {
+        // Use the Shorebird API that accepts base snapshot for linking.
+        return Dart_CreateIsolateGroupWithBaseSnapshot(
+            (*isolate_group_data)->GetAdvisoryScriptURI().c_str(),
+            (*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),
+            (*isolate_group_data)->GetIsolateSnapshot()->GetDataMapping(),
+            (*isolate_group_data)
+                ->GetIsolateSnapshot()
+                ->GetInstructionsMapping(),
+            base_snapshot->GetDataMapping(),
+            base_snapshot->GetInstructionsMapping(), flags, isolate_group_data,
+            isolate_data, error);
+      }
+#endif
       return Dart_CreateIsolateGroup(
           (*isolate_group_data)->GetAdvisoryScriptURI().c_str(),
           (*isolate_group_data)->GetAdvisoryScriptEntrypoint().c_str(),

engine/src/flutter/runtime/dart_isolate_group_data.cc

@@ -18,9 +18,11 @@ DartIsolateGroupData::DartIsolateGroupData(
     const ChildIsolatePreparer& child_isolate_preparer,
     const fml::closure& isolate_create_callback,
     const fml::closure& isolate_shutdown_callback,
-    std::shared_ptr<NativeAssetsManager> native_assets_manager)
+    std::shared_ptr<NativeAssetsManager> native_assets_manager,
+    fml::RefPtr<const DartSnapshot> base_snapshot)
     : settings_(settings),
       isolate_snapshot_(std::move(isolate_snapshot)),
+      base_snapshot_(std::move(base_snapshot)),
       advisory_script_uri_(std::move(advisory_script_uri)),
       advisory_script_entrypoint_(std::move(advisory_script_entrypoint)),
       child_isolate_preparer_(child_isolate_preparer),
@@ -41,6 +43,10 @@ fml::RefPtr<const DartSnapshot> DartIsolateGroupData::GetIsolateSnapshot()
   return isolate_snapshot_;
 }
 
+fml::RefPtr<const DartSnapshot> DartIsolateGroupData::GetBaseSnapshot() const {
+  return base_snapshot_;
+}
+
 const std::string& DartIsolateGroupData::GetAdvisoryScriptURI() const {
   return advisory_script_uri_;
 }

engine/src/flutter/runtime/dart_isolate_group_data.h

@@ -39,14 +39,19 @@ class DartIsolateGroupData : public PlatformMessageHandlerStorage {
       const ChildIsolatePreparer& child_isolate_preparer,
       const fml::closure& isolate_create_callback,
       const fml::closure& isolate_shutdown_callback,
-      std::shared_ptr<NativeAssetsManager> native_assets_manager = nullptr);
+      std::shared_ptr<NativeAssetsManager> native_assets_manager = nullptr,
+      fml::RefPtr<const DartSnapshot> base_snapshot = nullptr);
 
   ~DartIsolateGroupData();
 
   const Settings& GetSettings() const;
 
   fml::RefPtr<const DartSnapshot> GetIsolateSnapshot() const;
 
+  /// Returns the base snapshot for Shorebird linking support.
+  /// May be null if not using Shorebird or if no patch is active.
+  fml::RefPtr<const DartSnapshot> GetBaseSnapshot() const;
+
   const std::string& GetAdvisoryScriptURI() const;
 
   const std::string& GetAdvisoryScriptEntrypoint() const;
@@ -81,6 +86,7 @@ class DartIsolateGroupData : public PlatformMessageHandlerStorage {
   std::vector<std::shared_ptr<const fml::Mapping>> kernel_buffers_;
   const Settings settings_;
   const fml::RefPtr<const DartSnapshot> isolate_snapshot_;
+  const fml::RefPtr<const DartSnapshot> base_snapshot_;
   const std::string advisory_script_uri_;
   const std::string advisory_script_entrypoint_;
   mutable std::mutex child_isolate_preparer_mutex_;

engine/src/flutter/shell/common/shorebird/shorebird.cc

@@ -45,24 +45,38 @@ extern "C" __attribute__((weak)) unsigned long getauxval(unsigned long type) {
 }
 #endif
 
-// TODO(eseidel): I believe we need to leak these or we'll sometimes crash
-// when using the base snapshot in mixed mode.  This likely will not play
-// nicely with multi-engine support and will need to be refactored.
+#if SHOREBIRD_USE_INTERPRETER
+
+// Global references to the base (unpatched) snapshots from the App.framework.
+// These are process-global because:
+// 1. The Shorebird updater library is a process-global singleton with its own
+//    internal state. FileCallbacksImpl provides it access to the base snapshot
+//    data for patch generation/validation.
+// 2. The base snapshots are immutable (baked into the IPA) so sharing them
+//    across isolate groups is safe.
+// 3. GetBaseIsolateSnapshot() returns the isolate snapshot for use when
+//    creating isolate groups with Dart_CreateIsolateGroupWithBaseSnapshot(),
+//    which needs the base snapshot for linking patched code.
+//
+// Note: This design doesn't support multiple engines with different base
+// snapshots, but I'm not aware of any use cases for that on iOS.
 static fml::RefPtr<const DartSnapshot> vm_snapshot;
 static fml::RefPtr<const DartSnapshot> isolate_snapshot;
 
-void SetBaseSnapshot(Settings& settings) {
-  // These mappings happen to be to static data in the App.framework, but
-  // we still need to seem to hold onto the DartSnapshot objects to keep
-  // the mappings alive.
+void StoreBaseSnapshots(Settings& settings) {
+  // Create DartSnapshot objects that hold references to the symbol mappings
+  // in the App.framework. The snapshots are static data in the framework,
+  // but we need DartSnapshot objects to keep the NativeLibrary refs alive.
   vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings);
   isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings);
-  Shorebird_SetBaseSnapshots(isolate_snapshot->GetDataMapping(),
-                             isolate_snapshot->GetInstructionsMapping(),
-                             vm_snapshot->GetDataMapping(),
-                             vm_snapshot->GetInstructionsMapping());
 }
 
+fml::RefPtr<const DartSnapshot> GetBaseIsolateSnapshot() {
+  return isolate_snapshot;
+}
+
+#endif  // SHOREBIRD_USE_INTERPRETER
+
 class FileCallbacksImpl {
  public:
   static void* Open();
@@ -104,7 +118,9 @@ std::string GetValueFromYaml(const std::string& yaml, const std::string& key) {
   return "";
 }
 
-// FIXME: consolidate this with the other ConfigureShorebird
+/// Newer api, used by Desktop implementations.
+/// Does not directly manipulate Settings.
+// FIXME: Consolidate this with the other ConfigureShorebird() API.
 bool ConfigureShorebird(const ShorebirdConfigArgs& args,
                         std::string& patch_path) {
   patch_path = args.release_app_library_path;
@@ -201,6 +217,8 @@ bool ConfigureShorebird(const ShorebirdConfigArgs& args,
   return true;
 }
 
+/// Older api used by iOS and Android, directly manipulates Settings.
+// FIXME: Consolidate this with the other ConfigureShorebird() API.
 void ConfigureShorebird(std::string code_cache_path,
                         std::string app_storage_path,
                         Settings& settings,
@@ -251,15 +269,14 @@ void ConfigureShorebird(std::string code_cache_path,
                                  shorebird_yaml.c_str());
   }
 
-  // We've decided not to support synchronous updates on launch for now.
-  // It's a terrible user experience (having the app hang on launch) and
-  // instead we will provide examples of how to build a custom update UI
-  // within Dart, including updating as part of login, etc.
+  // We do not support synchronous updates on launch, it's a terrible UX.
+  // Users can implement custom check-for-updates using
+  // package:shorebird_code_push.
   // https://github.com/shorebirdtech/shorebird/issues/950
 
-  // We only set the base snapshot on iOS for now.
+  // We store the base snapshot on iOS for use when creating the isolate group.
 #if SHOREBIRD_USE_INTERPRETER
-  SetBaseSnapshot(settings);
+  StoreBaseSnapshots(settings);
 #endif
 
   shorebird_validate_next_boot_patch();
@@ -312,9 +329,17 @@ void ConfigureShorebird(std::string code_cache_path,
 }
 
 void* FileCallbacksImpl::Open() {
+#if SHOREBIRD_USE_INTERPRETER
   return SnapshotsDataHandle::createForSnapshots(*vm_snapshot,
                                                  *isolate_snapshot)
       .release();
+#else
+  // SnapshotsDataHandle exists on all platforms (for testing)but is only used
+  // on iOS. iOS patches are generated from just the Dart parts of the snapshot,
+  // excluding the Mach-O specific headers which contain dates and paths that
+  // make them change on every build.
+  return nullptr;
+#endif  // SHOREBIRD_USE_INTERPRETER
 }
 
 uintptr_t FileCallbacksImpl::Read(void* file,

engine/src/flutter/shell/common/shorebird/shorebird.h

@@ -2,15 +2,22 @@
 #define FLUTTER_SHELL_COMMON_SHOREBIRD_SHOREBIRD_H_
 
 #include "flutter/common/settings.h"
+#include "flutter/fml/memory/ref_ptr.h"
 #include "shell/platform/embedder/embedder.h"
 
 namespace flutter {
 
+class DartSnapshot;
+
+/// Version and build number of the release.
+/// Used by ShorebirdConfigArgs.
 struct ReleaseVersion {
   std::string version;
   std::string build_number;
 };
 
+/// Arguments for ConfigureShorebird.
+/// Used by Desktop implementations.
 struct ShorebirdConfigArgs {
   std::string code_cache_path;
   std::string app_storage_path;
@@ -30,18 +37,30 @@ struct ShorebirdConfigArgs {
         release_version(release_version) {}
 };
 
+/// Newer api, used by Desktop implementations.
+/// Does not directly manipulate Settings.
 bool ConfigureShorebird(const ShorebirdConfigArgs& args,
                         std::string& patch_path);
 
+/// Older api used by iOS and Android, directly manipulates Settings.
 void ConfigureShorebird(std::string code_cache_path,
                         std::string app_storage_path,
                         Settings& settings,
                         const std::string& shorebird_yaml,
                         const std::string& version,
                         const std::string& version_code);
 
+/// Used for reading app_id from shorebird.yaml.
+/// Exposed for testing.
 std::string GetValueFromYaml(const std::string& yaml, const std::string& key);
 
+#if SHOREBIRD_USE_INTERPRETER
+/// Returns the base isolate snapshot for Shorebird linking support.
+/// Must be called after ConfigureShorebird() has stored the base snapshots.
+/// May return null if not using Shorebird interpreter mode.
+fml::RefPtr<const DartSnapshot> GetBaseIsolateSnapshot();
+#endif  // SHOREBIRD_USE_INTERPRETER
+
 }  // namespace flutter
 
 #endif  // FLUTTER_SHELL_COMMON_SHOREBIRD_SHOREBIRD_H_