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

[Windows] Expose channel buffers 'resize' and 'overflow' control commands exposed by the control channel. #47158

Merged
merged 6 commits into from
Dec 14, 2023
Merged

[Windows] Expose channel buffers 'resize' and 'overflow' control commands exposed by the control channel. #47158

merged 6 commits into from
Dec 14, 2023

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Oct 20, 2023

Description

This PR adds two helper functions to invoke the 'resize' and 'overflow' commands exposed by the control channel.
See:

engine/lib/ui/channel_buffers.dart

Lines 302 to 309 in 93e8901

/// ## The control channel
///
/// A plugin can configure its channel's buffers by sending messages to the
/// control channel, `dev.flutter/channel-buffers` (see [kControlChannelName]).
///
/// There are two messages that can be sent to this control channel, to adjust
/// the buffer size and to disable the overflow warnings. See [handleMessage]
/// for details on these messages.

Implementation based on the discussion from #46998.

Related Issue

Windows implementation for flutter/flutter#132386

Tests

Adds 4 tests.

@bleroux bleroux force-pushed the windows_add_channel_buffer_helpers_resize_and_overflow branch from 5eb2181 to d7510bf Compare November 2, 2023 13:20
@bleroux bleroux requested a review from loic-sharma November 2, 2023 14:40
stuartmorgan-g
stuartmorgan-g suggested changes Nov 2, 2023
View reviewed changes
@bleroux bleroux force-pushed the windows_add_channel_buffer_helpers_resize_and_overflow branch 8 times, most recently from 9782c71 to 69e56a1 Compare November 9, 2023 14:46
bleroux
bleroux commented Nov 9, 2023
View reviewed changes
@chinmaygarde chinmaygarde changed the title [Windows] Expose channel buffers 'resize' and 'overflow' control comm… [Windows] Expose channel buffers 'resize' and 'overflow' control commands exposed by the control channel Nov 9, 2023
@chinmaygarde chinmaygarde changed the title [Windows] Expose channel buffers 'resize' and 'overflow' control commands exposed by the control channel [Windows] Expose channel buffers 'resize' and 'overflow' control commands exposed by the control channel. Nov 9, 2023
stuartmorgan-g
stuartmorgan-g suggested changes Nov 10, 2023
View reviewed changes
@bleroux bleroux force-pushed the windows_add_channel_buffer_helpers_resize_and_overflow branch from 26cb681 to c0e81b2 Compare November 13, 2023 15:11
@jonahwilliams
Copy link
Member

Friendly ping @stuartmorgan it looks like this needs another review.

stuartmorgan-g
stuartmorgan-g suggested changes Nov 30, 2023
View reviewed changes
@bleroux bleroux mentioned this pull request Dec 1, 2023
Open
2 tasks
@bleroux
Copy link
Contributor Author

bleroux commented Dec 1, 2023

Thanks for the feedback. I will pursue this work in two weeks (I won't have access to my Windows setup until December 11th).

@bleroux bleroux force-pushed the windows_add_channel_buffer_helpers_resize_and_overflow branch from c0e81b2 to 0c1609b Compare December 12, 2023 08:56
stuartmorgan-g
stuartmorgan-g suggested changes Dec 13, 2023
View reviewed changes
shell/platform/common/client_wrapper/core_implementations.cc Outdated

static constexpr char kControlChannelName[] = "dev.flutter/channel-buffers";
static constexpr char kResizeMethod[] = "resize";
static constexpr char kOverflowMethod[] = "overflow";
Copy link
Contributor

Choose a reason for hiding this comment

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

These three lines should be in an anonymous namespace (and thus don't need static to have internal linkage), rather than the internal namespace, since they don't need to be referenced outside of this file.

shell/platform/common/client_wrapper/core_implementations.cc Outdated
// Adjusts the number of messages that will get buffered when sending messages
// to channels that aren't fully set up yet. For example, the engine isn't
// running yet or the channel's message handler isn't set up on the Dart side
// yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and below: per the style guide, declaration comments belong at the declaration point (which is the header), not the definition point. That's true even for things that not intended for general use, like internal-namespaced functions or private class methods.

shell/platform/common/client_wrapper/include/flutter/basic_message_channel.h Outdated
@@ -14,6 +14,17 @@

namespace flutter {

namespace internal {
// Internal helpers functions used by BasicMessageChannel and MethodChannel.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: helper

@bleroux bleroux force-pushed the windows_add_channel_buffer_helpers_resize_and_overflow branch 2 times, most recently from 1ed71eb to c6e9fe9 Compare December 14, 2023 09:10
stuartmorgan-g
stuartmorgan-g approved these changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 14, 2023
@auto-submit auto-submit bot merged commit a565cea into flutter:main Dec 14, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 14, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 14, 2023
@engine-flutter-autoroll
a565cea25 [Windows] Expose channel buffers 'resize' and 'overflow' co…
563a433 
…ntrol commands exposed by the control channel. (flutter/engine#47158)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 14, 2023
@engine-flutter-autoroll
Roll Flutter Engine from caf33276468b to a565cea256c7 (1 revision) (#…
1454105 
…140163)

flutter/engine@caf3327...a565cea

2023-12-14 [email protected] [Windows] Expose channel buffers 'resize' and 'overflow' control commands exposed by the control channel. (flutter/engine#47158)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@bleroux bleroux deleted the windows_add_channel_buffer_helpers_resize_and_overflow branch December 15, 2023 20:35
@bleroux bleroux mentioned this pull request Dec 15, 2023
Closed
2 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@cbracken cbracken Awaiting requested review from cbracken

@yaakovschectman yaakovschectman Awaiting requested review from yaakovschectman

@loic-sharma loic-sharma Awaiting requested review from loic-sharma

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bleroux @jonahwilliams @stuartmorgan-g @loic-sharma @yaakovschectman