diff --git a/engine/src/flutter/runtime/shorebird/BUILD.gn b/engine/src/flutter/runtime/shorebird/BUILD.gn index facee07ba5cd1..81bf3f588b3f8 100644 --- a/engine/src/flutter/runtime/shorebird/BUILD.gn +++ b/engine/src/flutter/runtime/shorebird/BUILD.gn @@ -12,4 +12,9 @@ source_set("patch_cache") { "//flutter/fml", "//flutter/runtime:libdart", ] + + # For shorebird_report_launch_start() in updater.h + # The include path "third_party/updater/library/include/updater.h" is relative + # to //flutter/, which is already in the default include path. + include_dirs = [ "//flutter" ] } diff --git a/engine/src/flutter/runtime/shorebird/patch_cache.cc b/engine/src/flutter/runtime/shorebird/patch_cache.cc index d3dbe617ebfea..5610eb3393d60 100644 --- a/engine/src/flutter/runtime/shorebird/patch_cache.cc +++ b/engine/src/flutter/runtime/shorebird/patch_cache.cc @@ -4,9 +4,12 @@ #include "flutter/runtime/shorebird/patch_cache.h" +#include + #include "flutter/fml/logging.h" #include "flutter/fml/mapping.h" #include "flutter/runtime/shorebird/patch_mapping.h" +#include "third_party/updater/library/include/updater.h" namespace flutter { @@ -148,7 +151,18 @@ std::shared_ptr TryLoadFromPatch( FML_LOG(INFO) << "Loading symbol from patch: " << symbol_name; + // Report launch_start when we're actually about to use a patch. + // This is called at exactly the right time - right before the patched + // snapshot is loaded. We use std::once_flag to ensure it's only called + // once per process, and only for the first symbol (isolate data). + // This fixes the FlutterEngineGroup issue where report_launch_start was + // called too early (in ConfigureShorebird) before any patch was used. + static std::once_flag launch_start_flag; if (symbol == kIsolateDataSymbol) { + std::call_once(launch_start_flag, []() { + FML_LOG(INFO) << "Reporting launch start for patch"; + shorebird_report_launch_start(); + }); return PatchMapping::CreateIsolateData(cache_entry); } else { FML_CHECK(symbol == kIsolateInstructionsSymbol); diff --git a/engine/src/flutter/shell/common/shorebird/shorebird.cc b/engine/src/flutter/shell/common/shorebird/shorebird.cc index 81469a480f735..d040e07e0285d 100644 --- a/engine/src/flutter/shell/common/shorebird/shorebird.cc +++ b/engine/src/flutter/shell/common/shorebird/shorebird.cc @@ -181,25 +181,14 @@ bool ConfigureShorebird(const ShorebirdConfigArgs& args, FML_LOG(INFO) << "Shorebird updater: no active patch."; } - // We are careful only to report a launch start in the case where it's the - // first time we've configured shorebird this process. Otherwise we could end - // up in a case where we report a launch start, but never a completion (e.g. - // from package:flutter_work_manager which sometimes creates a FlutterEngine - // (and thus configures shorebird) but never runs it. The proper fix for this - // is probably to move the launch_start() call to be later in the lifecycle - // (when the snapshot is loaded and run, rather than when FlutterEngine is - // initialized). This "hack" will still have a problem where FlutterEngine is - // initialized but never run before the app is quit, could still cause us to - // suddenly mark-bad a patch that was never actually attempted to launch. + // Note: shorebird_report_launch_start() is now called from TryLoadFromPatch() + // in runtime/shorebird/patch_cache.cc, right before the patched snapshot is + // actually loaded. This fixes issues with FlutterEngineGroup and other cases + // where ConfigureShorebird() is called but no Shell is created. if (!init_result) { return false; } - // Once start_update_thread is called, the next_boot_patch* functions may - // change their return values if the shorebird_report_launch_failed - // function is called. - shorebird_report_launch_start(); - if (shorebird_should_auto_update()) { FML_LOG(INFO) << "Starting Shorebird update"; shorebird_start_update_thread(); @@ -294,25 +283,15 @@ void ConfigureShorebird(std::string code_cache_path, FML_LOG(INFO) << "Shorebird updater: no active patch."; } - // We are careful only to report a launch start in the case where it's the - // first time we've configured shorebird this process. Otherwise we could end - // up in a case where we report a launch start, but never a completion (e.g. - // from package:flutter_work_manager which sometimes creates a FlutterEngine - // (and thus configures shorebird) but never runs it. The proper fix for this - // is probably to move the launch_start() call to be later in the lifecycle - // (when the snapshot is loaded and run, rather than when FlutterEngine is - // initialized). This "hack" will still have a problem where FlutterEngine is - // initialized but never run before the app is quit, could still cause us to - // suddenly mark-bad a patch that was never actually attempted to launch. + // Note: shorebird_report_launch_start() is now called from TryLoadFromPatch() + // in runtime/shorebird/patch_cache.cc, right before the patched snapshot is + // actually loaded. This fixes issues with FlutterEngineGroup and other cases + // where ConfigureShorebird() is called but no Shell is created. + if (!init_result) { return; } - // Once start_update_thread is called, the next_boot_patch* functions may - // change their return values if the shorebird_report_launch_failed - // function is called. - shorebird_report_launch_start(); - if (shorebird_should_auto_update()) { FML_LOG(INFO) << "Starting Shorebird update"; shorebird_start_update_thread();