Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions engine/src/flutter/runtime/shorebird/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
}
14 changes: 14 additions & 0 deletions engine/src/flutter/runtime/shorebird/patch_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

#include "flutter/runtime/shorebird/patch_cache.h"

#include <mutex>

#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 {

Expand Down Expand Up @@ -148,7 +151,18 @@ std::shared_ptr<const fml::Mapping> 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);
Expand Down
39 changes: 9 additions & 30 deletions engine/src/flutter/shell/common/shorebird/shorebird.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Loading