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

Conversation

@loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Aug 16, 2022

The engine's Window references GetPointerType, which is supported only on Windows 8 and after. This causes Flutter apps to crash immediately on Windows 7.

This introduces WindowsProcTable to look up Windows APIs dynamically.

This bug was introduced in: #31594

Fixes flutter/flutter#109412

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.

@loic-sharma loic-sharma marked this pull request as ready for review August 16, 2022 23:59
Copy link
Contributor

@yaakovschectman yaakovschectman left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@moffatman moffatman left a comment

Choose a reason for hiding this comment

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

Looks good to me

MockTextInputManager* text_input_manager = new MockTextInputManager();
auto windows_proc_table = std::make_unique<MockWindowsProcTable>();
auto* text_input_manager = new MockTextInputManager();
std::unique_ptr<TextInputManager> text_input_manager_ptr(text_input_manager);
Copy link
Member

Choose a reason for hiding this comment

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

Can't remember why this is using new and not make_unique() but definitely feel free to clean it up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. I'll do it in a follow-up pull request. I need to merge this one back into the beta release branch, which is already has hairy merge conflicts 😅

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows 7] App crashes immediately if using master channel

5 participants