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

Win32: Add lifecycle channel support #29288

Closed
wants to merge 14 commits into from

Conversation

moko256
Copy link
Member

@moko256 moko256 commented Oct 21, 2021

This PR adds flutter/lifecycle channel support for win32 and winuwp(removed), and enable apps to receive window focused/minimized event.
This PR adds StringMessageCodec to shell/platform/common.

This PR will support part of flutter/flutter#103637

No changes in flutter/tests.

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.

Example (main.dart, modified from WidgetsBindingObserver example in docs):
This app displays last event and print it to stdout.

import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const AppLifecycleReactor(),
    );
  }
}

class AppLifecycleReactor extends StatefulWidget {
  const AppLifecycleReactor({Key? key}) : super(key: key);

  @override
  State<AppLifecycleReactor> createState() => _AppLifecycleReactorState();
}

class _AppLifecycleReactorState extends State<AppLifecycleReactor>
    with WidgetsBindingObserver {
  @override
  void initState() {
    super.initState();
    WidgetsBinding.instance!.addObserver(this);
  }

  @override
  void dispose() {
    WidgetsBinding.instance!.removeObserver(this);
    super.dispose();
  }

  AppLifecycleState? _notification;

  @override
  void didChangeAppLifecycleState(AppLifecycleState state) {
    print('Last notification: $state\n');
    setState(() {
      _notification = state;
    });
  }

  @override
  Widget build(BuildContext context) {
    return Text('Last notification: $_notification');
  }
}

@google-cla google-cla bot added the cla: yes label Oct 21, 2021
@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from 3d6b2bb to a7befa9 Compare October 23, 2021 06:10
@CaseyHillers CaseyHillers changed the base branch from master to main November 15, 2021 18:13
@godofredoc godofredoc changed the base branch from master to main November 24, 2021 07:13
@cbracken cbracken self-requested a review January 4, 2022 21:41
@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from a7befa9 to 2bd6b84 Compare February 17, 2022 22:09
@moko256
Copy link
Member Author

moko256 commented Feb 18, 2022

I resolved the conflicts and this is ready for review.

@ollyde
Copy link

ollyde commented Mar 13, 2022

We need this desperately. Do we have a commit for MacOS as well?

@moko256
Copy link
Member Author

moko256 commented Mar 13, 2022

@OllyDixon Sorry, this PR is for Windows only (and I don't have Mac now), but StringMessageCodec in this PR should help its implementation for MacOS and other platforms too.

@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from 2bd6b84 to b8baed8 Compare March 13, 2022 20:32
@moko256
Copy link
Member Author

moko256 commented Mar 13, 2022

@cbracken I ping softly (sorry if this was already triaged and is just low priority)

@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from b8baed8 to 5517fb1 Compare March 21, 2022 18:57
@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from 5517fb1 to 7c61755 Compare April 5, 2022 02:53
@moko256 moko256 changed the title Windows: Add lifecycle channel support Win32: Add lifecycle channel support Apr 5, 2022
@flutter-dashboard
Copy link

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.

@skia-gold
Copy link

Gold has detected about 27 new digest(s) on patchset 5.
View them at https://flutter-engine-gold.skia.org/cl/github/29288

@cbracken
Copy link
Member

Cross-referencing flutter/flutter#65061 since it's related.

@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from a648c2e to a141b01 Compare August 21, 2022 22:48
@Eldeloro1
Copy link

``

@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from 4738576 to adfa8f3 Compare September 5, 2022 07:42
@moko256 moko256 marked this pull request as ready for review September 5, 2022 16:33
@moko256
Copy link
Member Author

moko256 commented Sep 5, 2022

@cbracken @loic-sharma This is ready for review now. Please review this at your convenience.

Comment on lines +245 to +253
void FlutterWindow::OnSetFocus() {
has_focus_ = true;
SendLifecycleEvent();
}

void FlutterWindow::OnKillFocus() {
has_focus_ = false;
SendLifecycleEvent();
}
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to receive a WM_SETFOCUS or WM_KILLFOCUS message even after the window isn't visible? Would that scenario result in a pause event? If so, that seems undesirable.

@loic-sharma
Copy link
Member

Would it be possible to add an integration test to the engine using flutter_windows_unittests.cc and its corresponding main.dart fixture? In theory you should be able to post Windows messages and check you receive the expected lifecycle message in Dart.

@gspencergoog
Copy link
Contributor

This is a useful PR, and I'm aware that we very much need lifecycle event support, but I'm concerned that if we do this without a coherent strategy for all of the platforms that we'll end up with a fractured API for it that will require special handling for each platform on the framework side. Can we hold off on this just a little longer until we have a unified design?

@moko256
Copy link
Member Author

moko256 commented Nov 15, 2022

@gspencergoog I agree with that, but I don't have much time to fix this now.
I'm sorry, but I'll close this for now.

@moko256 moko256 closed this Nov 15, 2022
@loic-sharma
Copy link
Member

No worries! Your work will help inform the unified design, thank you for taking a first stab at this problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants