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

WIP - [Windows] Expose channel buffers 'resize' and 'overflow' control commands #46998

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Oct 17, 2023

Description

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

/// ## 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.

Related Issue

Windows implementation for flutter/flutter#132386

Tests

To do

@bleroux bleroux changed the title [Windows] Expose channel buffers 'resize' and 'overflow' control commands WIP - [Windows] Expose channel buffers 'resize' and 'overflow' control commands Oct 17, 2023
@bleroux
Copy link
Contributor Author

bleroux commented Oct 17, 2023

@stuartmorgan
While working on the Windows implementation for flutter/flutter#132386, I tried to figure out where to add the helpers functions.

On the other platforms there were somewhat obvious place to add this:

In this WIP PR (to be completed), I choose to add them to the core BinaryMessenger implementation, I'm not sure it is the best place.
I'm very interested in your view on this.

@bleroux bleroux force-pushed the windows_add_channel_buffer_utilities_resize_and_overflow branch from c314784 to 5cc3e7e Compare October 17, 2023 13:17
@bleroux bleroux force-pushed the windows_add_channel_buffer_utilities_resize_and_overflow branch from 5cc3e7e to 22dd8a8 Compare October 17, 2023 14:16
@stuartmorgan-g
Copy link
Contributor

We try to keep the channel API as consistent as possible across the platforms, so I would expect these to be exposed on the same conceptual object(s) as on the other platforms.

@bleroux
Copy link
Contributor Author

bleroux commented Oct 17, 2023

We try to keep the channel API as consistent as possible across the platforms, so I would expect these to be exposed on the same conceptual object(s) as on the other platforms.

I agree and i tried to find such a layer but it seems that for the Windows implementation it does not exist (probably because it was not required as the underlaying C++ API can be called directly).

For instance, when generating a plugin project for Windows, the channel and embedder implementations are directly exposed:

#include <flutter/method_channel.h>
#include <flutter/plugin_registrar_windows.h>
#include <flutter/standard_method_codec.h>

...

void WindowsPlugin::RegisterWithRegistrar(
    flutter::PluginRegistrarWindows *registrar) {
  auto channel =
      std::make_unique<flutter::MethodChannel<flutter::EncodableValue>>(
          registrar->messenger(), "windows_plugin",
          &flutter::StandardMethodCodec::GetInstance());
...

On the other platforms, the conceptual object(s) where I exposed those helper functions were:

  • Android: BasicMessageChannel and MethodChannel, Java implementation of the channel API.
  • iOS/macOS: FlutterBasicMessageChannel, ObjectiveC++ implementation of the channel API.
  • Linux: FlBinaryMessenger, GObject implementation of the binary messenger.

@stuartmorgan-g
Copy link
Contributor

For instance, when generating a plugin project for Windows, the channel and embedder implementations are directly exposed

What do you mean by "directly exposed"?

On the other platforms, the conceptual object(s) where I exposed those helper functions were:

  • Android: BasicMessageChannel and MethodChannel, Java implementation of the channel API.
  • iOS/macOS: FlutterBasicMessageChannel, ObjectiveC++ implementation of the channel API.

How are these conceptually different from flutter::MethodChannel?

@bleroux
Copy link
Contributor Author

bleroux commented Oct 18, 2023

What do you mean by "directly exposed"?

The way I understand the Android implementation is that it uses JNI to call the C++ implementation of the binary messenger (i have not look closely at the JNI calls so I might be wrong).

How are these conceptually different from flutter::MethodChannel?

I saw flutter::MethodChannel as a lower level implementation, but you are right that, conceptually, there are the same.

Great, if I understand well, instead of implementing Resizeand SetWarnsOnOverflowin flutter::BinaryMessenger, the path for this PR is to implement them in flutter::MethodChannel (and probably flutter::BasicMessageChannel). Is it the right path?

@stuartmorgan-g
Copy link
Contributor

What do you mean by "directly exposed"?

The way I understand the Android implementation is that it uses JNI to call the C++ implementation of the binary messenger (i have not look closely at the JNI calls so I might be wrong).

The C++ code here was written for Windows; it's not used by Android.

Great, if I understand well, instead of implementing Resizeand SetWarnsOnOverflowin flutter::BinaryMessenger, the path for this PR is to implement them in flutter::MethodChannel (and probably flutter::BasicMessageChannel). Is it the right path?

That matches my high-level expectation of what we would want. Without having looked closely at the details I don't know if there are other considerations that need to lead to a different outcome (as we unfortunately have with Linux).

@bleroux
Copy link
Contributor Author

bleroux commented Oct 19, 2023

Closing this WIP PR and I will file a new PR based on #46998 (comment)

@bleroux bleroux closed this Oct 19, 2023
@bleroux bleroux deleted the windows_add_channel_buffer_utilities_resize_and_overflow branch October 19, 2023 09:16
auto-submit bot pushed a commit that referenced this pull request Dec 14, 2023
…ands exposed by the control channel. (#47158)

## Description

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

https://github.com/flutter/engine/blob/93e8901490e78c7ba7e319cce4470d9c6478c6dc/lib/ui/channel_buffers.dart#L302-L309

Implementation based on the discussion from #46998.

## Related Issue

Windows implementation for flutter/flutter#132386

## Tests

Adds 4 tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants