Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Persist DartCallbackCache contents across launches #5947

Merged
bkonyi merged 8 commits into from
Aug 7, 2018
Merged

Persist DartCallbackCache contents across launches #5947

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dc0dc6b
Updated DartCallbackCache to write callback cache to disk which is
bkonyi Aug 6, 2018
8c9321a
Ensure cache isn't moved off disk in iOS + Formatting
bkonyi Aug 7, 2018
4b3db40
Merge branch 'master' into callback_cache_disk_conflicts
bkonyi Aug 7, 2018
04a6053
Formatting + licenses
bkonyi Aug 7, 2018
77bae66
Fixed licenses
bkonyi Aug 7, 2018
e27d219
Licenses...
bkonyi Aug 7, 2018
52981cc
Licenses.......
bkonyi Aug 7, 2018
c86a4cd
s/appRootPath/appStoragePath/g
bkonyi Aug 7, 2018
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
29 changes: 15 additions & 14 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/plugin/platform/Virtual
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterCallbackCache.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Headers/FlutterPluginAppLifeCycleDelegate.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterCallbackCache_Internal.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/FlutterPluginAppLifeCycleDelegate.mm
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.h
FILE: ../../../flutter/shell/platform/darwin/ios/framework/Source/accessibility_text_entry.mm
Expand Down Expand Up @@ -635,13 +636,17 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

====================================================================================================
LIBRARY: engine
ORIGIN: ../../../flutter/fml/time/time_delta_unittest.cc + ../../../third_party/tonic/LICENSE
ORIGIN: ../../../flutter/lib/ui/painting/image.cc + ../../../LICENSE
TYPE: LicenseType.bsd
FILE: ../../../flutter/fml/export.h
FILE: ../../../flutter/fml/time/time_delta_unittest.cc
FILE: ../../../flutter/fml/time/time_point_unittest.cc
FILE: ../../../flutter/lib/ui/painting/image.cc
FILE: ../../../flutter/lib/ui/painting/image.h
FILE: ../../../flutter/shell/common/switches.cc
FILE: ../../../flutter/shell/common/switches.h
FILE: ../../../flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterMain.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java
----------------------------------------------------------------------------------------------------
Copyright 2017 The Fuchsia Authors. All rights reserved.
Copyright 2013 The Chromium Authors. All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
Expand Down Expand Up @@ -672,17 +677,13 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

====================================================================================================
LIBRARY: engine
ORIGIN: ../../../flutter/lib/ui/painting/image.cc + ../../../LICENSE
ORIGIN: ../../../garnet/LICENSE
TYPE: LicenseType.bsd
FILE: ../../../flutter/lib/ui/painting/image.cc
FILE: ../../../flutter/lib/ui/painting/image.h
FILE: ../../../flutter/shell/common/switches.cc
FILE: ../../../flutter/shell/common/switches.h
FILE: ../../../flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterMain.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java
FILE: ../../../flutter/fml/export.h
FILE: ../../../flutter/fml/time/time_delta_unittest.cc
FILE: ../../../flutter/fml/time/time_point_unittest.cc
----------------------------------------------------------------------------------------------------
Copyright 2013 The Chromium Authors. All rights reserved.
Copyright 2017 The Fuchsia Authors. All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
Expand Down
101 changes: 100 additions & 1 deletion lib/ui/plugins/callback_cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,39 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/lib/ui/plugins/callback_cache.h"
#include <fstream>
#include <iterator>

#include "flutter/fml/build_config.h"
#include "flutter/fml/logging.h"
#include "flutter/fml/paths.h"
#include "flutter/lib/ui/plugins/callback_cache.h"
#include "third_party/rapidjson/rapidjson/document.h"
#include "third_party/rapidjson/rapidjson/stringbuffer.h"
#include "third_party/rapidjson/rapidjson/writer.h"
#include "third_party/tonic/converter/dart_converter.h"

using rapidjson::Document;
using rapidjson::StringBuffer;
using rapidjson::Writer;
using tonic::ToDart;

namespace blink {

static const char* kHandleKey = "handle";
static const char* kRepresentationKey = "representation";
static const char* kNameKey = "name";
static const char* kClassNameKey = "class_name";
static const char* kLibraryPathKey = "library_path";
static const char* kCacheName = "flutter_callback_cache.json";
std::mutex DartCallbackCache::mutex_;
std::string DartCallbackCache::cache_path_;
std::map<int64_t, DartCallbackRepresentation> DartCallbackCache::cache_;

void DartCallbackCache::SetCachePath(const std::string& path) {
cache_path_ = fml::paths::JoinPaths({path, kCacheName});
}

Dart_Handle DartCallbackCache::GetCallback(int64_t handle) {
std::lock_guard<std::mutex> lock(mutex_);
auto iterator = cache_.find(handle);
Expand All @@ -34,6 +56,7 @@ int64_t DartCallbackCache::GetCallbackHandle(const std::string& name,

if (cache_.find(hash) == cache_.end()) {
cache_[hash] = {name, class_name, library_path};
SaveCacheToDisk();

@jason-simmons jason-simmons Aug 7, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the lifecycle of the cache file? Is the cache file or any of its entries ever deleted?

Would anything within the cache become stale if the application's code is updated but the cache is not purged?

@bkonyi bkonyi Aug 7, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the cache is persistent and entries aren't deleted, although we could verify all the callbacks in the cache actually exist when we restore and remove the stale entries.

}
return hash;
}
Expand All @@ -48,6 +71,82 @@ DartCallbackCache::GetCallbackInformation(int64_t handle) {
return nullptr;
}

void DartCallbackCache::SaveCacheToDisk() {
// Cache JSON format
// [
// {
// "hash": 42,
// "representation": {
// "name": "...",
// "class_name": "...",
// "library_path": "..."
// }
// },
// {
// ...
// }
// ]
StringBuffer s;
Writer<StringBuffer> writer(s);
writer.StartArray();
for (auto iterator = cache_.begin(); iterator != cache_.end(); ++iterator) {
int64_t hash = iterator->first;
DartCallbackRepresentation cb = iterator->second;
writer.StartObject();
writer.Key(kHandleKey);
writer.Int64(hash);
writer.Key(kRepresentationKey);
writer.StartObject();
writer.Key(kNameKey);
writer.String(cb.name.c_str());
writer.Key(kClassNameKey);
writer.String(cb.class_name.c_str());
writer.Key(kLibraryPathKey);
writer.String(cb.library_path.c_str());
writer.EndObject();
writer.EndObject();
}
writer.EndArray();

std::ofstream output(cache_path_);
output << s.GetString();
output.close();
}

void DartCallbackCache::LoadCacheFromDisk() {
std::lock_guard<std::mutex> lock(mutex_);

// Don't reload the cache if it's already populated.
if (!cache_.empty()) {
return;
}
std::ifstream input(cache_path_);
if (!input) {
return;
}
std::string cache_contents{std::istreambuf_iterator<char>(input),
std::istreambuf_iterator<char>()};
Document d;
d.Parse(cache_contents.c_str());
if (d.HasParseError() || !d.IsArray()) {
FML_LOG(WARNING) << "Could not parse callback cache, aborting restore";
// TODO(bkonyi): log and bail (delete cache?)
return;
}
const auto entries = d.GetArray();
for (auto it = entries.begin(); it != entries.end(); ++it) {
const auto root_obj = it->GetObject();
const auto representation = root_obj[kRepresentationKey].GetObject();

const int64_t hash = root_obj[kHandleKey].GetInt64();
DartCallbackRepresentation cb;
cb.name = representation[kNameKey].GetString();
cb.class_name = representation[kClassNameKey].GetString();
cb.library_path = representation[kLibraryPathKey].GetString();
cache_[hash] = cb;
}
}

Dart_Handle DartCallbackCache::LookupDartClosure(
const std::string& name,
const std::string& class_name,
Expand Down
10 changes: 8 additions & 2 deletions lib/ui/plugins/callback_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
#include "flutter/fml/synchronization/thread_annotations.h"
#include "third_party/dart/runtime/include/dart_api.h"

#define DART_CALLBACK_INVALID_HANDLE -1

namespace blink {

typedef struct {
Expand All @@ -26,6 +24,9 @@ typedef struct {

class DartCallbackCache {
public:
static void SetCachePath(const std::string& path);
static std::string GetCachePath() { return cache_path_; }

static int64_t GetCallbackHandle(const std::string& name,
const std::string& class_name,
const std::string& library_path)
Expand All @@ -36,12 +37,17 @@ class DartCallbackCache {
static std::unique_ptr<DartCallbackRepresentation> GetCallbackInformation(
int64_t handle) FML_LOCKS_EXCLUDED(mutex_);

static void LoadCacheFromDisk() FML_LOCKS_EXCLUDED(mutex_);

private:
static Dart_Handle LookupDartClosure(const std::string& name,
const std::string& class_name,
const std::string& library_path);

static void SaveCacheToDisk() FML_EXCLUSIVE_LOCKS_REQUIRED(mutex_);

static std::mutex mutex_;
static std::string cache_path_;

static std::map<int64_t, DartCallbackRepresentation> cache_
FML_GUARDED_BY(mutex_);
Expand Down
11 changes: 9 additions & 2 deletions shell/platform/android/flutter_main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "flutter/fml/message_loop.h"
#include "flutter/fml/paths.h"
#include "flutter/fml/platform/android/jni_util.h"
#include "flutter/lib/ui/plugins/callback_cache.h"
#include "flutter/runtime/dart_vm.h"
#include "flutter/runtime/start_up.h"
#include "flutter/shell/common/shell.h"
Expand Down Expand Up @@ -44,7 +45,8 @@ void FlutterMain::Init(JNIEnv* env,
jclass clazz,
jobject context,
jobjectArray jargs,
jstring bundlePath) {
jstring bundlePath,
jstring appRootPath) {

@zanderso zanderso Aug 6, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who has write access to appRootPath? Could application code accidentally clobber the callback cache?

@bkonyi bkonyi Aug 6, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is only visible to the internals of FlutterMain.java which only sets it to the result of PathUtils.getFilesDir(applicationContext); so it shouldn't be possible for the application to clobber the cache unless it's really trying to (e.g., by deleting the cache file directly).

std::vector<std::string> args;
args.push_back("flutter");
for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) {
Expand All @@ -56,6 +58,11 @@ void FlutterMain::Init(JNIEnv* env,

settings.assets_path = fml::jni::JavaStringToString(env, bundlePath);

// Restore the callback cache.
blink::DartCallbackCache::SetCachePath(
fml::jni::JavaStringToString(env, appRootPath));
blink::DartCallbackCache::LoadCacheFromDisk();

if (!blink::DartVM::IsRunningPrecompiledCode()) {
// Check to see if the appropriate kernel files are present and configure
// settings accordingly.
Expand Down Expand Up @@ -97,7 +104,7 @@ bool FlutterMain::Register(JNIEnv* env) {
{
.name = "nativeInit",
.signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/"
"lang/String;)V",
"lang/String;Ljava/lang/String;)V",
.fnPtr = reinterpret_cast<void*>(&Init),
},
{
Expand Down
3 changes: 2 additions & 1 deletion shell/platform/android/flutter_main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class FlutterMain {
jclass clazz,
jobject context,
jobjectArray jargs,
jstring bundlePath);
jstring bundlePath,
jstring appRootPath);

FML_DISALLOW_COPY_AND_ASSIGN(FlutterMain);
};
Expand Down
4 changes: 4 additions & 0 deletions shell/platform/android/io/flutter/util/PathUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
import android.content.Context;

public final class PathUtils {
public static String getFilesDir(Context applicationContext) {
return applicationContext.getFilesDir().getPath();
}

public static String getDataDirectory(Context applicationContext) {
return applicationContext.getDir("flutter", Context.MODE_PRIVATE).getPath();
}
Expand Down
5 changes: 3 additions & 2 deletions shell/platform/android/io/flutter/view/FlutterMain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,9 @@ public static void ensureInitializationComplete(Context applicationContext, Stri
}

String appBundlePath = findAppBundlePath(applicationContext);
String appRootPath = PathUtils.getFilesDir(applicationContext);
nativeInit(applicationContext, shellArgs.toArray(new String[0]),
appBundlePath);
appBundlePath, appRootPath);

sInitialized = true;
} catch (Exception e) {
Expand All @@ -229,7 +230,7 @@ public static void ensureInitializationComplete(Context applicationContext, Stri
}
}

private static native void nativeInit(Context context, String[] args, String bundlePath);
private static native void nativeInit(Context context, String[] args, String bundlePath, String appRootPath);

@jason-simmons jason-simmons Aug 7, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change appRootPath to something reflecting that this path is writable (e.g. "storagePath" or "writablePath"?)

imho appRootPath sounds like a path where the app's own assets would live, inviting confusion with bundlePath

@bkonyi bkonyi Aug 7, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, will do.

private static native void nativeRecordStartTimestamp(long initTimeMillis);

/**
Expand Down
1 change: 1 addition & 0 deletions shell/platform/darwin/ios/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ shared_library("create_flutter_framework_dylib") {
"framework/Source/FlutterAppDelegate.mm",
"framework/Source/FlutterAppDelegate_Internal.h",
"framework/Source/FlutterCallbackCache.mm",
"framework/Source/FlutterCallbackCache_Internal.h",
"framework/Source/FlutterChannels.mm",
"framework/Source/FlutterCodecs.mm",
"framework/Source/FlutterDartProject.mm",
Expand Down
13 changes: 11 additions & 2 deletions shell/platform/darwin/ios/framework/Headers/FlutterPlugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ NS_ASSUME_NONNULL_BEGIN
*/
- (BOOL)application:(UIApplication*)application
didFinishLaunchingWithOptions:(NSDictionary*)launchOptions;

/**
Called if this plugin has been registered for `UIApplicationDelegate` callbacks.

- Returns: `NO` if this plugin vetoes application launch.
*/
- (BOOL)application:(UIApplication*)application
willFinishLaunchingWithOptions:(NSDictionary*)launchOptions;

/**
Called if this plugin has been registered for `UIApplicationDelegate` callbacks.
*/
Expand Down Expand Up @@ -293,8 +302,8 @@ NS_ASSUME_NONNULL_BEGIN
@end

/**
Implement this in the `UIAppDelegate` of your app to enable Flutter plugins to register themselves to the application
life cycle events.
Implement this in the `UIAppDelegate` of your app to enable Flutter plugins to register themselves
to the application life cycle events.
*/
@protocol FlutterAppLifeCycleProvider
- (void)addApplicationLifeCycleDelegate:(NSObject<FlutterPlugin>*)delegate;
Expand Down
Loading