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

Win32: Add lifecycle channel support #29288

Closed
moko256 wants to merge 14 commits into from
Closed

Win32: Add lifecycle channel support #29288

moko256 wants to merge 14 commits into from

Conversation

@moko256
Copy link
Contributor

@moko256 moko256 commented Oct 21, 2021 •
edited
Loading

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
Contributor 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
Contributor 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
Contributor 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

flutter-dashboard bot commented Apr 25, 2022

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.

@moko256 moko256 mentioned this pull request May 12, 2022
Closed
4 tasks
@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from 7c61755 to a648c2e Compare May 12, 2022 17:10
@skia-gold
Copy link

skia-gold commented May 12, 2022

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

@cbracken cbracken requested a review from loic-sharma June 30, 2022 07:19
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 6, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 7, 2022
View reviewed changes
loic-sharma
loic-sharma reviewed Jul 7, 2022
View reviewed changes
@cbracken
Copy link
Member

cbracken commented Jul 19, 2022

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

@justinmc justinmc mentioned this pull request Jul 26, 2022
Merged
@moko256 moko256 force-pushed the moko256_windows_lifecycle_channel branch from a648c2e to a141b01 Compare August 21, 2022 22:48
@Eldeloro1
Copy link

Eldeloro1 commented Aug 21, 2022

``

@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
Contributor Author

moko256 commented Sep 5, 2022

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

@justinmc justinmc mentioned this pull request Sep 6, 2022
Open
loic-sharma
loic-sharma reviewed Sep 8, 2022
View reviewed changes
shell/platform/windows/flutter_window.cc
Comment on lines +245 to +253
void FlutterWindow::OnSetFocus() {
has_focus_ = true;
SendLifecycleEvent();
}

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

@loic-sharma loic-sharma Sep 8, 2022

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

loic-sharma commented Sep 8, 2022

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

gspencergoog commented Nov 14, 2022

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
Contributor 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

loic-sharma commented Nov 15, 2022

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cbracken cbracken Awaiting requested review from cbracken

@yaakovschectman yaakovschectman Awaiting requested review from yaakovschectman

1 more reviewer

@loic-sharma loic-sharma loic-sharma left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes platform-windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@moko256 @ollyde @skia-gold @loic-sharma @cbracken @Eldeloro1 @gspencergoog