Skip to content

Skip injecting Bonjour settings when port publication is disabled #136562

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
Merged
auto-submit merged 6 commits into from
Oct 17, 2023
Merged

Skip injecting Bonjour settings when port publication is disabled #136562

auto-submit merged 6 commits into from
Oct 17, 2023

Conversation

@vashworth
Copy link
Contributor

@vashworth vashworth commented Oct 13, 2023 •
edited
Loading

Some of our tests in CI are triggering the NSLocalNetworkUsageDescription dialog when they're not supposed to (#129836) since it's disabled via flags (--no-publish-port for flutter/flutter and --disable-vm-service-publication for flutter/engine).

Normally, we inject NSLocalNetworkUsageDescription (and other bonjour settings) to the Info.plist during the project build for debug and profile mode since by default we publish the VM Service port over mDNS.

To help diagnose the issue, though, this PR changes it so that we don't inject NSLocalNetworkUsageDescription (and other bonjour settings) when port publication is disabled since it shouldn't be needed. Hopefully, this will give us better error messages or cause the app to crash and end the test early (rather than timeout after 30 minutes).

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, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@github-actions github-actions bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Oct 13, 2023
@vashworth
Copy link
Contributor Author

vashworth commented Oct 16, 2023

Ran a couple of ios__start_up tests in presubmit to ensure nothing is broken:
https://ci.chromium.org/ui/p/flutter/builders/try/Mac_ios%20complex_layout_ios__start_up/4/overview
https://ci.chromium.org/ui/p/flutter/builders/try/Mac_ios%20flutter_gallery_ios__start_up/12/overview

@vashworth vashworth marked this pull request as ready for review October 17, 2023 15:43
christopherfujino
christopherfujino reviewed Oct 17, 2023
View reviewed changes
christopherfujino
christopherfujino approved these changes Oct 17, 2023
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2023
@auto-submit auto-submit bot merged commit 0383d8b into flutter:master Oct 17, 2023
@vashworth vashworth mentioned this pull request Oct 17, 2023
Closed
vashworth added a commit that referenced this pull request Oct 17, 2023
@vashworth
Revert "Skip injecting Bonjour settings when port publication is disa…
7a07939 
…bled (#136562)"

This reverts commit 0383d8b.
@vashworth vashworth added the revert Autorevert PR (with "Reason for revert:" comment) label Oct 17, 2023
@auto-submit auto-submit bot mentioned this pull request Oct 17, 2023
Merged
8 tasks
@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Oct 17, 2023
auto-submit bot pushed a commit that referenced this pull request Oct 17, 2023
Revert "Skip injecting Bonjour settings when port publication is disa…
c0e227a 
…bled (#136562)"

This reverts commit 0383d8b.
auto-submit bot added a commit that referenced this pull request Oct 17, 2023
@auto-submit
Reverts "Skip injecting Bonjour settings when port publication is dis…
54c0a35 
…abled" (#136750)

Reverts #136562
Initiated by: vashworth
This change reverts the following previous change:
Original Description:
Some of our tests in CI are triggering the `NSLocalNetworkUsageDescription` dialog when they're not supposed to (#129836) since it's disabled via flags (`--no-publish-port` for flutter/flutter and `--disable-vm-service-publication` for flutter/engine).

Normally, we inject `NSLocalNetworkUsageDescription` (and other bonjour settings) to the Info.plist during the project build for debug and profile mode since by default they will publish the VM Service port over mDNS.

To help diagnose the issue, though, this PR changes it so that we don't inject `NSLocalNetworkUsageDescription` (and other bonjour settings) when port publication is disabled since it shouldn't be needed. Hopefully, this will give us better error messages or cause the app to crash and end the test early (rather than timeout after 30 minutes).
@dkwingsmt dkwingsmt added revert Autorevert PR (with "Reason for revert:" comment) and removed revert Autorevert PR (with "Reason for revert:" comment) labels Oct 17, 2023
@vashworth vashworth mentioned this pull request Oct 17, 2023
Merged
8 tasks
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
31bed0f 
…bled (flutter/flutter#136562)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
f5df104 
…bled (flutter/flutter#136562)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
faedd37 
…bled (flutter/flutter#136562)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
84bb52b 
…bled (flutter/flutter#136562)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
c55ac94 
…bled (flutter/flutter#136562)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
aa9d9cf 
…bled (flutter/flutter#136562)
auto-submit bot pushed a commit that referenced this pull request Oct 18, 2023
@vashworth
[Reland] Skip injecting Bonjour settings when port publication is dis…
1599cbe 
…abled (#136751)

Reland of #136562 with fixes.
@auto-submit auto-submit bot mentioned this pull request Oct 18, 2023
Merged
8 tasks
auto-submit bot added a commit that referenced this pull request Oct 18, 2023
@auto-submit
Reverts "[Reland] Skip injecting Bonjour settings when port publicati…
c32ffa6 
…on is disabled" (#136839)

Reverts #136751
Initiated by: vashworth
This change reverts the following previous change:
Original Description:
Reland of #136562 with fixes.
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 18, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023
@engine-flutter-autoroll
0383d8b Skip injecting Bonjour settings when port publication is disa…
9ae6e3a 
…bled (flutter/flutter#136562)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 19, 2023
@engine-flutter-autoroll
Manual roll Flutter from 6eea6e2 to 189196d (44 revisions) (#5178)
09c6b11 
Manual roll Flutter from 6eea6e2 to 189196d (44 revisions)

Manual roll requested by tarrinneal@google.com

flutter/flutter@6eea6e2...189196d

2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from ab86c53c19cd to 6caee3236d37 (2 revisions) (flutter/flutter#136834)
2023-10-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Reland] Skip injecting Bonjour settings when port publication is disabled" (flutter/flutter#136839)
2023-10-18 gspencergoog@users.noreply.github.com Convert menus to use OverlayPortal (flutter/flutter#130534)
2023-10-18 15619084+vashworth@users.noreply.github.com [Reland] Skip injecting Bonjour settings when port publication is disabled (flutter/flutter#136751)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0eff5d191856 to ab86c53c19cd (1 revision) (flutter/flutter#136832)
2023-10-18 47866232+chunhtai@users.noreply.github.com Fixes ability to call nextFocus() on a node to focus its descendant (flutter/flutter#136773)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from b778a07f8ae9 to 0eff5d191856 (6 revisions) (flutter/flutter#136829)
2023-10-18 robert.ancell@canonical.com Implement GApplication:shutdown so a Flutter developer knows where to put code that should occur on application shutdown. (flutter/flutter#136780)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7f37c9b181af to b778a07f8ae9 (1 revision) (flutter/flutter#136818)
2023-10-18 tessertaha@gmail.com Fix `Slider` `onChanged` callback order & never calls `onChangeStart` on  `SliderInteraction.slideOnly` allowed interaction (flutter/flutter#136720)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 78026b4003fe to 7f37c9b181af (1 revision) (flutter/flutter#136817)
2023-10-18 838@mail.com [Feat] Stroke color for Slider value indicator (flutter/flutter#135986)
2023-10-18 76067278+aakash-pamnani@users.noreply.github.com Fixed : Empty Rows shown at last page in Paginated data table (flutter/flutter#132646)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 46923fd39032 to 78026b4003fe (1 revision) (flutter/flutter#136814)
2023-10-18 engine-flutter-autoroll@skia.org Roll Packages from d439062 to 14aa69e (1 revision) (flutter/flutter#136813)
2023-10-18 dacoharkes@google.com Don't build native assets in `flutter build bundle` (flutter/flutter#136641)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from c9c9684e03a3 to 46923fd39032 (1 revision) (flutter/flutter#136795)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0c1c29271e8b to c9c9684e03a3 (1 revision) (flutter/flutter#136792)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1de09d13e708 to 0c1c29271e8b (3 revisions) (flutter/flutter#136789)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6fc36e61a99a to 1de09d13e708 (1 revision) (flutter/flutter#136785)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3f818efff3c5 to 6fc36e61a99a (1 revision) (flutter/flutter#136781)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2eef9b43cfb4 to 3f818efff3c5 (3 revisions) (flutter/flutter#136777)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5df7af34a718 to 2eef9b43cfb4 (4 revisions) (flutter/flutter#136774)
2023-10-18 christopherfujino@gmail.com Flutter preview device (flutter/flutter#135639)
2023-10-18 gildasiogx@gmail.com Add findChildIndexCallback examples (flutter/flutter#133469)
2023-10-18 109253501+pdblasi-google@users.noreply.github.com Adds API for performing semantics actions in tests (flutter/flutter#132598)
2023-10-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from e57b5bac4244 to 5df7af34a718 (2 revisions) (flutter/flutter#136772)
2023-10-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from f9f937e51080 to e57b5bac4244 (3 revisions) (flutter/flutter#136768)
2023-10-17 andrewrkolos@gmail.com clean up `--dart-define-from-file` option tests (flutter/flutter#135980)
2023-10-17 sokolovskyi.konstantin@gmail.com GoldenFileComparators should dispose created Image objects.  (flutter/flutter#136716)
2023-10-17 lc1990linux@gmail.com make integration_test_driver_extended.dart support writeResponseData--(done) (flutter/flutter#128382)
2023-10-17 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.6.0 to 4.1.1 (flutter/flutter#136762)
2023-10-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 289f29b1ad00 to f9f937e51080 (1 revision) (flutter/flutter#136755)
2023-10-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 659e68a097b5 to 289f29b1ad00 (1 revision) (flutter/flutter#136752)
2023-10-17 aran@arandonohue.com Support --web-header option for flutter run (flutter/flutter#136297)
2023-10-17 34871572+gmackall@users.noreply.github.com Add Android 14 physical devices to firebase tests (flutter/flutter#136736)
2023-10-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 62a90a91cee3 to 659e68a097b5 (3 revisions) (flutter/flutter#136746)
2023-10-17 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Skip injecting Bonjour settings when port publication is disabled" (flutter/flutter#136750)
2023-10-17 katelovett@google.com Revert "[SingleChildScrollView] Correct the offset pixels if it is out of range during layout" (flutter/flutter#136744)
2023-10-17 15619084+vashworth@users.noreply.github.com Skip injecting Bonjour settings when port publication is disabled (flutter/flutter#136562)
2023-10-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3ecbe924a598 to 62a90a91cee3 (2 revisions) (flutter/flutter#136739)
2023-10-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0a4d8b99a95b to 3ecbe924a598 (3 revisions) (flutter/flutter#136732)
2023-10-17 32242716+ricardoamador@users.noreply.github.com Reenable NexusLowRes API 29 (flutter/flutter#136686)
2023-10-17 32242716+ricardoamador@users.noreply.github.com Reenable the nexus 6p tests (flutter/flutter#136689)
...
@vashworth vashworth mentioned this pull request Mar 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vashworth @christopherfujino @dkwingsmt