[macOS] Add lookupKeyForAsset to FlutterPluginRegistrar by zhongwuzw · Pull Request #37421 · flutter/engine

zhongwuzw · 2022-11-08T12:09:45Z

Fixes flutter/flutter#47681

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

jmagman · 2022-11-09T21:21:04Z

cc @cbracken

chinmaygarde · 2022-11-17T21:14:30Z

Ping @cbracken.

cbracken · 2022-11-18T00:49:26Z

Apologies; was out last week. Taking a look!

chinmaygarde · 2022-12-01T21:11:54Z

Gentle ping @cbracken

flutter-dashboard · 2022-12-27T21:21:25Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h

shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm

shell/platform/darwin/macos/framework/Headers/FlutterViewController.h

shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h

shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm

shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

chinmaygarde · 2023-01-12T21:09:08Z

Ping @a-wallen

chinmaygarde · 2023-01-19T21:15:50Z

@jmagman Are you able to review this in @a-wallen s absence?

a-wallen · 2023-01-19T21:24:00Z

@chinmaygarde this PR has my LGTM, but @stuartmorgan left review comments that haven't been addressed.

chinmaygarde · 2023-01-26T21:18:05Z

@zhongwuzw Any updates on the test for the class method?

zhongwuzw · 2023-01-28T03:09:31Z

@zhongwuzw Any updates on the test for the class method?

I'll update the feedback after Chinese new year :)

jmagman · 2023-02-21T18:08:19Z

FAILED: obj/flutter/shell/platform/darwin/macos/framework/Source/flutter_framework_source.FlutterDartProject.o 
/Volumes/Work/s/w/ir/cache/goma/client/gomacc  ../../buildtools/mac-x64/clang/bin/clang++ -MD -MF obj/flutter/shell/platform/darwin/macos/framework/Source/flutter_framework_source.FlutterDartProject.o.d -DFLUTTER_FRAMEWORK -DFLUTTER_ENGINE_NO_PROTOTYPES -DUSE_OPENSSL=1 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_LIBCPP_DISABLE_AVAILABILITY=1 -D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DSK_TYPEFACE_FACTORY_CORETEXT -DSK_GL -DSK_METAL -DSK_SUPPORT_GPU=1 -DSK_VULKAN -DSK_CODEC_DECODES_JPEG -DSK_ENCODE_JPEG -DSK_CODEC_DECODES_PNG -DSK_ENCODE_PNG -DSK_CODEC_DECODES_WEBP -DSK_ENCODE_WEBP -DSK_HAS_WUFFS_LIBRARY -DFLUTTER_RUNTIME_MODE_DEBUG=1 -DFLUTTER_RUNTIME_MODE_PROFILE=2 -DFLUTTER_RUNTIME_MODE_RELEASE=3 -DFLUTTER_RUNTIME_MODE_JIT_RELEASE=4 -DDART_LEGACY_API=\[\[deprecated\]\] -DFLUTTER_RUNTIME_MODE=1 -DFLUTTER_JIT_RUNTIME=1 -DVULKAN_HPP_NO_EXCEPTIONS=1 -DIMPELLER_SUPPORTS_RENDERING=1 -DIMPELLER_ENABLE_METAL=1 -DIMPELLER_ENABLE_OPENGLES=1 -DIMPELLER_ENABLE_VULKAN=1 -DSK_ENABLE_DUMP_GPU -DSK_DISABLE_AAA -DSK_LEGACY_IGNORE_DRAW_VERTICES_BLEND_WITH_NO_SHADER -DSK_DISABLE_LEGACY_SHADERCONTEXT -DSK_DISABLE_LOWP_RASTER_PIPELINE -DSK_FORCE_RASTER_PIPELINE_BLITTER -DSK_METAL_WAIT_UNTIL_SCHEDULED -DSK_DISABLE_EFFECT_DESERIALIZATION -DSK_ENABLE_SKSL -DSK_ENABLE_PRECOMPILE -DSK_USE_PERFETTO -DSK_LEGACY_LAYER_BOUNDS_EXPANSION -DSK_ENABLE_API_AVAILABLE -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DU_ENABLE_TRACING=1 -DU_ENABLE_RESOURCE_TRACING=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DRAPIDJSON_HAS_STDSTRING -DRAPIDJSON_HAS_CXX11_RANGE_FOR -DRAPIDJSON_HAS_CXX11_RVALUE_REFS -DRAPIDJSON_HAS_CXX11_TYPETRAITS -DRAPIDJSON_HAS_CXX11_NOEXCEPT -DVK_USE_PLATFORM_METAL_EXT -DVMA_DYNAMIC_VULKAN_FUNCTIONS=0 -DVMA_STATIC_VULKAN_FUNCTIONS=0 -DFLUTTER_API_SYMBOL_PREFIX=Embedder -DFLUTTER_NO_EXPORT -DSHELL_ENABLE_SOFTWARE -DSHELL_ENABLE_GL -DSHELL_ENABLE_VULKAN -DSHELL_ENABLE_METAL -I../.. -Igen -I../../third_party/libcxx/include -I../../third_party/libcxxabi/include -I../../build/secondary/third_party/libcxx/config -I../../third_party/vulkan_memory_allocator/include -I../../third_party/vulkan-deps/vulkan-headers/src/include -I../../flutter -Igen/flutter -I../../third_party/flatbuffers/include -I../../third_party/skia -I../../flutter/third_party -I../../third_party/dart/runtime -I../../third_party/dart/runtime/include -I../../flutter/third_party/txt/src -I../../third_party/harfbuzz/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/rapidjson -I../../third_party/rapidjson/include -I../../third_party/angle/include -I../../flutter/third_party/accessibility -I../../flutter/shell/platform/embedder -I../../flutter/shell/platform/darwin/common/framework/Headers -isysroot /Volumes/Work/s/w/ir/cache/osx_sdk/XCode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.0.sdk -mmacosx-version-min=10.14.0 -fno-strict-aliasing -fstack-protector-all -arch x86_64 -fcolor-diagnostics -Wall -Wextra -Wendif-labels -Werror -Wno-missing-field-initializers -Wno-unused-parameter -Wno-unused-but-set-parameter -Wno-unused-but-set-variable -Wno-implicit-int-float-conversion -Wno-c99-designator -Wno-deprecated-copy -Wno-psabi -Wno-unqualified-std-cast-call -Wno-non-c-typedef-for-linkage -Wno-range-loop-construct -Wunguarded-availability -Wno-deprecated-declarations -fvisibility=hidden -Wstring-conversion -Wnewline-eof -O2 -fno-ident -fdata-sections -ffunction-sections -g2 -Wno-newline-eof  -Werror=overriding-method-mismatch -Werror=undeclared-selector -fobjc-arc -fvisibility-inlines-hidden -fobjc-call-cxx-cdtors -std=c++17 -fno-rtti -nostdinc++ -nostdinc++ -fvisibility=hidden -fno-exceptions  -c ../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm -o obj/flutter/shell/platform/darwin/macos/framework/Source/flutter_framework_source.FlutterDartProject.o
../../flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject.mm:95:12: error: property 'enableMirrors' not found on object of type 'FlutterDartProject *'
  if (self.enableMirrors) {
           ^
1 error generated.

chinmaygarde · 2023-02-21T18:09:49Z

Perhaps 650db7a.

cbracken · 2023-02-21T18:14:56Z

Yes - I removed that flag last week. It's been deprecated for eons and was present only for an internal customer. Rebasing should resolve.

zhongwuzw · 2023-04-06T04:48:55Z

@jmagman @stuartmorgan Please review again :)

shell/platform/darwin/common/framework/Headers/FlutterDartProject.h

shell/platform/darwin/common/framework/Source/FlutterDartProjectHelper.mm

shell/platform/darwin/ios/framework/Source/FlutterDartProject_Internal.h

shell/platform/darwin/common/framework/Source/FlutterDartProjectHelper.mm

shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm

…s_asset_lookup_key

zhongwuzw · 2023-04-14T06:43:03Z

@stuartmorgan @jmagman Please review again, thanks :)

zhongwuzw · 2023-04-24T08:44:18Z

@stuartmorgan @jmagman Friendly ping, I think it's ready to review again, any review feedback?

gspencergoog · 2023-04-27T18:14:22Z

@stuartmorgan @jmagman Do you have time to take another look at this PR? (Desktop Triage)

jmagman

Sorry for the long delay, LGTM

auto-submit · 2023-05-02T03:14:24Z

auto label is removed for flutter/engine, pr: 37421, due to This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan}, please make the needed changes and resubmit this PR.
You have project association MEMBER and need 1 more review(s) in order to merge this PR.

Merge guidelines: You need at least one approved review if you are already a MEMBER or two member reviews if you are not a MEMBER before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

stuartmorgan-g

LGTM (with one question), thanks!

stuartmorgan-g · 2023-05-02T10:53:50Z

shell/platform/darwin/common/BUILD.gn


  public = framework_shared_headers

+  public += [ "framework/Source/FlutterNSBundleUtils.h" ]


This doesn't cause it to be in the public headers directory of the actual final framework bundle, does it?

it's just be shared between macos and iOS framework internal.Not affect final public headers.

…ter/engine#37421)

flutter/engine@a687d62...46d5ce4 2023-05-02 zhongwuzw@qq.com [macOS] Add lookupKeyForAsset to FlutterPluginRegistrar (flutter/engine#37421) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jsimmons@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

flutter-dashboard bot added platform-ios platform-macos labels Nov 8, 2022

zhongwuzw requested a review from stuartmorgan-g November 8, 2022 12:15

jmagman requested a review from cbracken November 9, 2022 21:20

jmagman removed the platform-ios label Dec 7, 2022

stuartmorgan-g suggested changes Jan 3, 2023

View reviewed changes

jmagman reviewed Jan 3, 2023

View reviewed changes

shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h Show resolved Hide resolved

shell/platform/darwin/macos/framework/Headers/FlutterDartProject.h Show resolved Hide resolved

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm Outdated Show resolved Hide resolved

loic-sharma requested a review from a-wallen January 6, 2023 00:38

zhongwuzw added 2 commits January 20, 2023 12:10

[macOS] Add lookupKeyForAsset to FlutterPluginRegistrar

2d43d42

clang tidy

7541807

zhongwuzw force-pushed the add_macos_asset_lookup_key branch from 0967005 to 7541807 Compare January 20, 2023 04:57

Remove NS_ASSUME_NONNULL_BEGIN macro

d1b8d0d

zhongwuzw requested review from stuartmorgan-g and removed request for a-wallen January 30, 2023 02:28

cbenhagen mentioned this pull request Jan 30, 2023

Can't load video from assets ollyde/flutter_macos_video_player#11

Open

loic-sharma added the affects: desktop label Feb 9, 2023

zhongwuzw requested a review from jmagman February 20, 2023 06:42

Merge branch 'main' into add_macos_asset_lookup_key

d2faa10

flutter deleted a comment from flutter-dashboard bot Apr 6, 2023

cyanglaz mentioned this pull request Apr 8, 2023

[mac] Build mac framework with app extension flag #41011

Merged

8 tasks

zhongwuzw requested a review from jmagman April 12, 2023 01:44

stuartmorgan-g suggested changes Apr 13, 2023

View reviewed changes

cyanglaz mentioned this pull request Apr 13, 2023

Cannot create a Flutter macOS app extension: 'FlutterEngineInitialize' returned 'kInvalidArguments'. The assets path in the Flutter project arguments was missing. flutter/flutter#124755

Open

Apply reviews

4e0925b

zhongwuzw requested a review from chinmaygarde as a code owner April 14, 2023 03:44

zhongwuzw removed the request for review from chinmaygarde April 14, 2023 03:45

zhongwuzw force-pushed the add_macos_asset_lookup_key branch from 9fcfb2d to 4e0925b Compare April 14, 2023 03:54

zhongwuzw added 2 commits April 14, 2023 12:14

Merge commit 'c9a4d69c20591a536284db50b0a2c2a202f92e3f' into add_maco…

d253d3e

…s_asset_lookup_key

Add licence

3b3e6aa

zhongwuzw requested a review from stuartmorgan-g April 14, 2023 06:42

jmagman approved these changes May 2, 2023

View reviewed changes

zhongwuzw added the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2023

stuartmorgan-g approved these changes May 2, 2023

View reviewed changes

zhongwuzw added the autosubmit Merge PR when tree becomes green via auto submit App label May 2, 2023

auto-submit bot merged commit 46d5ce4 into flutter:main May 2, 2023

engine-flutter-autoroll mentioned this pull request May 2, 2023

Roll Flutter Engine from a687d62ff631 to 46d5ce4784b8 (1 revision) flutter/flutter#125874

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 2, 2023

46d5ce4 [macOS] Add lookupKeyForAsset to FlutterPluginRegistrar (flut…

0583d86

…ter/engine#37421)


		public = framework_shared_headers

		public += [ "framework/Source/FlutterNSBundleUtils.h" ]

Comments

Conversation

zhongwuzw commented Nov 8, 2022

Pre-launch Checklist

Uh oh!

jmagman commented Nov 9, 2022

Uh oh!

chinmaygarde commented Nov 17, 2022

Uh oh!

cbracken commented Nov 18, 2022

Uh oh!

chinmaygarde commented Dec 1, 2022

Uh oh!

flutter-dashboard bot commented Dec 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chinmaygarde commented Jan 12, 2023

Uh oh!

chinmaygarde commented Jan 19, 2023

Uh oh!

a-wallen commented Jan 19, 2023

Uh oh!

chinmaygarde commented Jan 26, 2023 • edited by zhongwuzw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhongwuzw commented Jan 28, 2023

Uh oh!

jmagman commented Feb 21, 2023

Uh oh!

chinmaygarde commented Feb 21, 2023

Uh oh!

cbracken commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhongwuzw commented Apr 6, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhongwuzw commented Apr 14, 2023

Uh oh!

zhongwuzw commented Apr 24, 2023

Uh oh!

gspencergoog commented Apr 27, 2023

Uh oh!

jmagman left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented May 2, 2023

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g May 2, 2023

Choose a reason for hiding this comment

Uh oh!

zhongwuzw May 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

chinmaygarde commented Jan 26, 2023 •

edited by zhongwuzw

Loading

cbracken commented Feb 21, 2023 •

edited

Loading

zhongwuzw May 2, 2023 •

edited

Loading