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

Remove engine dependency of LifecycleChannel #127

Merged

Conversation

swift-kim
Copy link
Member

@swift-kim swift-kim commented Jun 25, 2021

Sends lifecycle messages through a BasicMessageChannel instead of using FlutterTizenEngine::SendPlatformMessage.

I implemented StringCodec from scratch because there was no equivalent of the framework's StringCodec in cpp_client_wrapper. StandardCodecSerializer already has support for the string data type but its serialization format is slightly different from StringCodec's.

@flutter-tizen/maintainers How do you feel about this change? I can discard this change if it doesn't make very much sense.

@swift-kim swift-kim force-pushed the lifecycle-channel branch from 3ced96a to 4f0df04 Compare June 25, 2021 07:21
@bwikbs
Copy link
Member

bwikbs commented Jun 28, 2021

First of all, Thank you for your efforts to improve code. 👏 (I should do too... but it's not easy... 😄 )

In my case, I am not sure which one is better, so I referred to other implementation...but I still don't know. 😢
I'm really curious what other people think regarding this.

@swift-kim swift-kim force-pushed the lifecycle-channel branch from 4f0df04 to ad78711 Compare June 28, 2021 01:41
@swift-kim
Copy link
Member Author

There are a few more "Channel" implementations that depend on FlutterTizenEngine, and I'm planning to refactor them as well after this change.

Copy link

@bbrto21 bbrto21 left a comment

Choose a reason for hiding this comment

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

It looks similar to the Android implementation and In my view, this is better.

Copy link

@pkosko pkosko left a comment

Choose a reason for hiding this comment

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

LGTM

@bwikbs
Copy link
Member

bwikbs commented Jun 29, 2021

It looks similar to the Android implementation and In my view, this is

@bbrto21 Why do you think so? I'm just curious... I want to know even a small or minor part.

@bbrto21
Copy link

bbrto21 commented Jun 29, 2021

It looks similar to the Android implementation and In my view, this is

@bbrto21 Why do you think so? I'm just curious... I want to know even a small or minor part.

I've just seen these
https://github.com/flutter/engine/blob/6a87e0b558e45ae0e7d3369757512bb3c7c151fa/shell/platform/android/io/flutter/plugin/common/StringCodec.java#L17
https://github.com/flutter/engine/blob/6a87e0b558e45ae0e7d3369757512bb3c7c151fa/shell/platform/android/io/flutter/embedding/engine/systemchannels/LifecycleChannel.java#L20

@swift-kim
Copy link
Member Author

@bbrto21 FYI, it's also consistent with the Dart side definition of the lifecycle channel.

https://github.com/flutter/flutter/blob/2.2.1/packages/flutter/lib/src/services/system_channels.dart#L210-L223

@bbrto21 bbrto21 merged commit cafd059 into flutter-tizen:flutter-2.2.1-tizen Jun 29, 2021
swift-kim added a commit that referenced this pull request Sep 27, 2021
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request Nov 14, 2021
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request Dec 9, 2021
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request Dec 17, 2021
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request Feb 7, 2022
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request Feb 11, 2022
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request May 12, 2022
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim added a commit that referenced this pull request Aug 5, 2022
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
swift-kim pushed a commit that referenced this pull request Sep 1, 2022
swift-kim added a commit that referenced this pull request Sep 1, 2022
* Remove engine dependency of LifecycleChannel

* Implement StringSerializer

* Remove StringSerializer and implement StringCodec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants