Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Load macOS dart bundle by URL fallback #22979

Merged
merged 2 commits into from
Dec 10, 2020
Merged

Load macOS dart bundle by URL fallback #22979

merged 2 commits into from
Dec 10, 2020

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Dec 10, 2020 •
edited
Loading

Description

flutter/flutter#71764 (reverted) will stop explicitly linking the App.framework into the macOS app, which means [NSBundle bundleWithIdentifier:[FlutterDartProject defaultBundleIdentifier]] returns null because the bundle isn't loaded (no longer LC_LOAD_DYLIB), and so the assets aren't found:

2020-12-08T14:22:03.476319: stdout: [ +341 ms] 2020-12-08 14:22:03.475 flutter_gallery[38930:24214860] Failed to find path for "flutter_assets"
2020-12-08T14:22:03.477822: stdout: [   +1 ms] 2020-12-08 14:22:03.475 flutter_gallery[38930:24214860] Failed to find path for "flutter_assets"
stdout: [        ] embedder.cc (764): 'FlutterEngineInitialize' returned 'kInvalidArguments'. The assets path in the Flutter project arguments was missing.
stdout: [        ] 2020-12-08 14:22:03.475 flutter_gallery[38930:24214860] Failed to initialize Flutter engine: error 2
2020-12-08T15:22:20.615880: Task execution finished

Instead, fallback macOS to detecting the dart bundle by path to Contents/Frameworks/App.framework.

Related Issues

Unblock flutter/flutter#71764.

Tests

I'm not sure how to explicitly test this, but hot_mode_dev_cycle_macos_target__benchmark will pass on top of flutter/flutter#71764 after this change.

@jmagman jmagman self-assigned this Dec 10, 2020
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@google-cla google-cla bot added the cla: yes label Dec 10, 2020
@dnfield
Copy link
Contributor

dnfield commented Dec 10, 2020

Test failures look like flakes. We should file infra presubmit flake bugs for them.

dnfield
dnfield approved these changes Dec 10, 2020
View reviewed changes
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM - it would be nice if we had a test for this in this repo, but as long as there's a test covering it in the devicelab that should be fine.

@jmagman
Copy link
Member Author

jmagman commented Dec 10, 2020

Test failures look like flakes. We should file infra presubmit flake bugs for them.

Filed flutter/flutter#72047

stuartmorgan
stuartmorgan approved these changes Dec 10, 2020
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM with nit.

@jmagman
Copy link
Member Author

jmagman commented Dec 10, 2020

I changed only be the macOS change, I'll put out another PR for iOS.

@jmagman jmagman changed the title Load dart bundle by URL fallback Load macOS dart bundle by URL fallback Dec 10, 2020
@jmagman jmagman mentioned this pull request Dec 10, 2020
Merged
@jmagman jmagman merged commit 3b9937a into flutter:master Dec 10, 2020
@jmagman jmagman deleted the app-load branch December 10, 2020 20:51
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 11, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 11, 2020
@engine-flutter-autoroll
3b9937a Load macOS dart bundle by URL fallback (flutter/engine#22979)
e57bee7
dnfield pushed a commit to flutter/flutter that referenced this pull request Dec 11, 2020
@engine-flutter-autoroll
Roll Engine from 4797b06 to 4794d04 (23 revisions) (#72147)
7c2cfff 
* 3a30ae3 Fix ios voiceover (for safari >13.4) (flutter/engine#22965)

* 4338849 Replace g_object_weak_ref with g_object_add_weak_pointer

* 3b9937a Load macOS dart bundle by URL fallback (flutter/engine#22979)

* 96927bb add ffi_struct_patch.dart to libraries.yaml (flutter/engine#23000)

* 2efc7c1 Set SkPath::setIsVolatile based on whether the path survives at least two frames (flutter/engine#22620)

* bb81b95 Allow Tile mode for blur filter and add new decal TileMode (flutter/engine#22982)

* 9df2157 Load iOS dart bundle by URL fallback (flutter/engine#22997)

* 7647fdb Roll Skia from 22f80a60b17f to 6b07e0eb497c (26 revisions) (flutter/engine#23005)

* 062cbd8 Freiling warmup memory (flutter/engine#22984)

* 1646966 Revert "Freiling warmup memory (#22984)" (flutter/engine#23007)

* 50d830a [web] Do not reset 'cursor' in PersistedPlatformView. (flutter/engine#22977)

* 6ebf5c3 Roll Dart SDK from e4c9b06267d3 to a4e6fe145bf7 (2 revisions) (flutter/engine#23006)

* 14c8c24 [web] Fix regression in foreground style (flutter/engine#22999)

* 6678efa Implement SystemSound.play

* fb769a4 Roll Fuchsia Linux SDK from rnN_X2o75... to ESzmO-yOF... (flutter/engine#23010)

* b424356 Roll Skia from 6b07e0eb497c to f7cce2b243b2 (6 revisions) (flutter/engine#23018)

* 56035c7 Roll Fuchsia Linux SDK from ESzmO-yOF... to K4cPd0-Xd... (flutter/engine#23020)

* cb4a2ef Roll Skia from f7cce2b243b2 to b0cb8372c1ef (3 revisions) (flutter/engine#23021)

* cc8c9d4 Roll Skia from b0cb8372c1ef to 5284e96599a8 (2 revisions) (flutter/engine#23023)

* 8e9a943 Roll Dart SDK from a4e6fe145bf7 to c287db6bf232 (2 revisions) (flutter/engine#23024)

* 714b543 Roll Fuchsia Mac SDK from OUQEzH1oE... to a9yuHfriB... (flutter/engine#23025)

* d50cdda Roll Dart SDK from c287db6bf232 to 2553a84fe438 (1 revision) (flutter/engine#23026)

* 4794d04 Roll Skia from 5284e96599a8 to f7fdf1aa2911 (1 revision) (flutter/engine#23027)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 14, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 14, 2020
@engine-flutter-autoroll
3b9937a Load macOS dart bundle by URL fallback (flutter/engine#22979)
6da3b1d
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 14, 2020
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 14, 2020
@engine-flutter-autoroll
3b9937a Load macOS dart bundle by URL fallback (flutter/engine#22979)
018c1bc
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 14, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 14, 2020
@engine-flutter-autoroll
3b9937a Load macOS dart bundle by URL fallback (flutter/engine#22979)
06f721d
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 15, 2020
@engine-flutter-autoroll
3b9937a Load macOS dart bundle by URL fallback (flutter/engine#22979)
f720a02
dnfield added a commit that referenced this pull request Dec 15, 2020
@dnfield
Revert "Load macOS dart bundle by URL fallback (#22979)"
383ff9d 
This reverts commit 3b9937a.
This was referenced Dec 15, 2020
Merged
Merged
@jmagman jmagman mentioned this pull request Jan 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

@dnfield dnfield dnfield approved these changes

Assignees

@jmagman jmagman

Labels
cla: yes platform-macos
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmagman @dnfield @stuartmorgan