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.

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

[video_player] Platform interface changes to fix Android rotation for videos recorded in landscapeRight #1

Conversation

KyleFin
Copy link
Owner

@KyleFin KyleFin commented Dec 21, 2021

Platform interface changes required for flutter#3820 (review)

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#60327

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

  • [x ] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x ] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x ] I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • [x ] I signed the CLA.
  • [x ] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • [x ] I listed at least one issue that this PR fixes in the description above.
  • [x ] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • [x ] I updated CHANGELOG.md to add a description of the change.
  • [x ] I updated/added relevant documentation (doc comments with ///).
  • [x ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • [x ] All existing and new tests are passing.

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

cpboyd and others added 30 commits September 4, 2021 21:26
Reverts #4288 which published a breaking change as if it were a non-breaking change.

The discussion in that PR prior to it landing was incorrect, because adding a new parameter
with a default value is non-breaking *only for clients*. It is breaking for subclasses that
override it, and the purpose of the platform interface is for implementations to subclass it
and override everything.
It's possible for jobs to conclude in a "neutral" state; see https://github.com/flutter/plugins/runs/3534917860 for instance. Currently this causes "release" to be red when it shouldn't be.
Uses the new generic plugins recipe to add a number of new Windows tests
that have recently gained tool support, but not yet added to the LUCI
builds:
- New targets:
  - The "build all plugins" test (master and stable)
  - UWP build tests (master only, since UWP is not on stable yet)
  - Tool tests (master only with existing tool tests on Linux)
  - Replacement versions of the existing builders but with names that
    match the Cirrus naming to make the parallels in testing easier
    to understand in the GitHub UI and configs.
- Modification of existing targets:
  - Adds Windows native unit tests to the existing script.
Fixes the target names in LUCI to not use '+', since it's not supported.

To better align the names between the two different infrastructures,
also updates the task naming in cirrus.yml to not use +. It also
standardizes the naming form across both systems as:
`<Host platform> <target platform>-<test_name> <extra info>`
where:
- `<Host platform>` is always there on LUCI, where required, but only
  added where it's potentially ambiguous on Cirrus.
- `<target platform>' is omitted when the test is not target-platform
  specific.
- `<test_name>' uses underscores, which is consistent with
  flutter/flutter (and with Cirrus step naming).
- `<extra info>` is only explicitly set (to the channel) on LUCI; Cirrus
  automatically adds channel info there due to the way `matrix` works.
Now that the builders have propagated, enable all the new tests and remove the obsolete versions.
This brings the native Android unit tests in line with the policy of having tests that we expect all plugins to have—unless there's a very specific reason to opt them out—fail when missing instead of skipping when missing, to help guard against errors where we silently fail to run tests we think we are running.

Adds an explicit exclusion list covering the plugins that have a reason to be opted out.

Android portion of flutter/flutter#85469
…4298)

This commit:

* uses the zIndex attribute when converting Circle geometry objects.
* ensures that the getScreenCoordinate method works as expected on the web platform.
  * adds tests that can use a fully-rendered Google Map (see projection_test.dart)
    * changes the initialization flow of the web Google Map, so the Controller is only returned to the main plugin when it's ready to work.

In order to test the getScreenCoordinate method, the Controller of a fully-rendered map must be available on the test, so we can retrieve information from an actual map instance. While working on this, it was observed that the Controller was being sent to the programmer before it was truly ready (while the map was still initializing).

Instead of littering the test with imprecise timeouts that may make these tests slower (and flakier) than needed, this PR also changes the initialization process of a GMap slightly so when its Controller is returned to the user of the plugin (onPlatformViewCreated method call), it is truly ready.

For this: 

* Controller.init is immediately called after the controller is created, 
* The plugin waits for the first onTilesloaded event coming from the JS SDK, and then 
* The Controller is sent to the user

This change happens within "private" sections of the plugin, so programmers using the plugin "normally" shouldn't notice any difference whatsoever (only that the GMap might load slightly faster, and the onPlatformViewCreated callback might be firing a few hundred milliseconds later).
Brings iOS and macOS into alignment with the other platforms, where
having unit tests set up is required.

- For deprecated plugins with no tests, `--exclude`s them, as on other platforms.
- For `quick_actions` and `share`, which have integration tests but no unit tests,
  sets up the unit test scaffolding. (This is done for `share` even though it's
  deprecated since unlike other platforms, iOS/macOS runs both native tests in the 
  same command, and setting up a special way to exclude just units tests for that
  one case would be much more effort.)

Fixes flutter/flutter#85469
…4218)

TMPDIR is a standard variable on UNIX/Linux systems, and is often used in containers such as Flatpak to redirect to a temporary folder inside a sandbox. This allows not to make hard bindings to the /tmp directory

Fixes flutter/flutter#87742
Uses the background script annotation, since & no longer works.
…eption when being replaced in the `VideoPlayer` (#4344)

* fix: do not removeListener if VideoPlayerController is already disposed

Co-authored-by: David Iglesias Teixeira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment