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

[webview_flutter] Add webview_flutter_tizen package #54

Merged
merged 6 commits into from
Apr 8, 2021

Conversation

seungsoo47
Copy link
Contributor

Co-authored-by: MuHong Byun mh.byun@samsung.com
Co-authored-by: Seungsoo Lee seungsoo47.lee@samsung.com

@swift-kim
Copy link
Member

What are those .so files? Which source code were they built from?

@seungsoo47
Copy link
Contributor Author

The file is a library for Tizen lightweight web engine. Because the file is not included in Tizen Studio yet, we decided to include them in the plugin.

@seungsoo47
Copy link
Contributor Author

I am sorry, I found unnecessary files. I will remove them.

@swift-kim
Copy link
Member

  1. Where is the source code for the library? How can you build it?
  2. You provided .so files for the arm architecture only. Isn't this plugin supported on emulators? If not, why?

the file is not included in Tizen Studio yet

  1. Is the library expected to be part of the Tizen SDK? If so, is there any detailed plan?
  2. Why is the library necessary for this plugin to function? What makes it differ from the built-in web engine shipped with existing Tizen devices?

@seungsoo47
Copy link
Contributor Author

  1. Please refer to link below:
    https://review.tizen.org/gerrit/gitweb?p=platform/upstream/lightweight-web-engine.git;a=tree;h=refs/heads/tizen;hb=refs/heads/tizen

  2. The library only supports the ARM architecture. Initially, we had implemented General Buffer-based webview, but after that, we changed to FBO-based webview to improve performance.
    This guarantees better performance, but there was a problem not running on the emulator. The current solution is not satisfactory for both device and emulator, but we will try to find the solution ASAP.

  3. I am not sure. But if the library is not included, one way is to include x86 library in the plugin.

  4. We needed to implement a webview that can share memory buffer to draw web pages on the display, also we didn't know how flutter and dart work with built-in webview on devices.
    So first we tried to merge the lightweight web engine that we made on Tizen. Later, We will extend to new webview with Tizen builtin web engine based on chromium.

@swift-kim
Copy link
Member

swift-kim commented Mar 29, 2021

Including a ELF file in source tree is not a good practice. Would it be possible to let the user download the file directly from download.tizen.org or the Releases tab of this repo? (This idea was abandoned.)

Also why does the so file has the .mobile suffix in its name? Is the file expected work with any profile (wearable/tv)?

@seungsoo47 seungsoo47 force-pushed the webview_tizen branch 2 times, most recently from b3d8c5a to eeb05e5 Compare March 30, 2021 03:07
@seungsoo47
Copy link
Contributor Author

I have updated the file's suffix from'.mobile' to'.flutter'.

Comment on lines +34 to +37
virtual void DispatchCompositionUpdateEvent(const char* str,
int size) override;
virtual void DispatchCompositionEndEvent(const char* str, int size) override;

Copy link
Contributor

Choose a reason for hiding this comment

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

When I tried to run example on Tizen device (flutter-tizen run -v), two errors occurred:

           "/home/r.walczyna/tizen-studio/tools/smart-build-interface/../llvm-10/bin/clang++" -c "src/webview_flutter_tizen_plugin.cc" -o "Debug/src/webview_flutter_tizen_plugin.o" -DTIZEN_DEPRECATION -DDEPRECATION_WARNING
-DTIZEN_DEPRECATION
[...]
           In file included from src/webview_flutter_tizen_plugin.cc:16:
           src/webview.h:34:16: error: 'DispatchCompositionUpdateEvent' marked 'override' but does not override any member functions
             virtual void DispatchCompositionUpdateEvent(const char* str,
                          ^
           src/webview.h:36:16: error: 'DispatchCompositionEndEvent' marked 'override' but does not override any member functions
             virtual void DispatchCompositionEndEvent(const char* str, int size) override;
                          ^
           2 errors generated.

After removing not needed overrides it has built successful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that you need to update flutter-tizen. Actually, I have updated "flutter_platform_view.h" in flutter engine and the file should been applied on flutter-tizen cache side.

Copy link
Member

@swift-kim swift-kim Apr 1, 2021

Choose a reason for hiding this comment

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

@seungsoo47 An engine release containing this patch has just been released yesterday. The embedder API versioning is a quite complicated problem because a plugin can have only a constraint on the Flutter SDK version (below) but not the Tizen embedder version. I'll tell you a bit more about this matter later.

environment:
  flutter: ">=2.0.1"

Copy link
Member

@swift-kim swift-kim left a comment

Choose a reason for hiding this comment

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

I believe libraries that a plugin depends on (liblightweight-web-engine.flutter.so in this case) should not be manually copied from the plugin directory into a project directory by the developer. Instead, they should automatically be copied by the flutter-tizen tool into a build output directory directly (this can be done in TizenPlugins of tizen_build_target.dart). I or @HakkyuKim will prepare a fix for this if you agree.

packages/webview_flutter/.metadata Outdated Show resolved Hide resolved
packages/webview_flutter/example/.metadata Outdated Show resolved Hide resolved
packages/webview_flutter/CHANGELOG.md Outdated Show resolved Hide resolved
packages/webview_flutter/example/pubspec.yaml Show resolved Hide resolved
packages/webview_flutter/example/tizen/tizen-manifest.xml Outdated Show resolved Hide resolved
packages/webview_flutter/pubspec.yaml Outdated Show resolved Hide resolved
@seungsoo47
Copy link
Contributor Author

I believe libraries that a plugin depends on (liblightweight-web-engine.flutter.so in this case) should not be manually copied from the plugin directory into a project directory by the developer. Instead, they should automatically be copied by the flutter-tizen tool into a build output directory directly (this can be done in TizenPlugins of tizen_build_target.dart). I or @HakkyuKim will prepare a fix for this if you agree.

Thank you for your help. Please correct the copy issue. And I will update results for other issues.

* This PR is created from the PR below:
2bfc038 [webview_flutter] Update README and pubspec (flutter-tizen#53)
bdd6a15 [webview_flutter] Enable intergation test (flutter-tizen#52)
18da2a6 [webview_flutter] Update code by flutter version-up (flutter-tizen#49)
1b6eeed [webview_flutter] Handle CompositionEvent (flutter-tizen#47)
791511e [webview_flutter] Optimize tbm allocation (flutter-tizen#42)
7059ce2 [webview_flutter] Add cookie and scroll APIs (flutter-tizen#40)
c5a8930 Add some APIs (flutter-tizen#37)
3feae1d [webview_flutter] Add NavigationDelegate and onWebResourceError (flutter-tizen#36)
42b63f8 [webview_flutter] Add removeJavascriptChannels API and Callbacks (flutter-tizen#31)
749930e [webview_flutter] Update LWE (flutter-tizen#28)
9e26386 [webview_flutter] Implement addJavascriptChannels() (flutter-tizen#27)
6a34ae0 [webview_flutter] Update LWE (flutter-tizen#26)
a438093 [webview_flutter] Add evaluateJavascript() function (flutter-tizen#25)
0d2c271 [webview_flutter] Update webview to be focused (flutter-tizen#19)
b02b245 Enable IMF on Webview (flutter-tizen#17)
571fa4a [webview_flutter_dev] Change webview interface && embedding lwe.so file (flutter-tizen#15)
3413964 [webview_flutter_dev] Enable gl compositor at LWE (flutter-tizen#14)
e988cf0 Change method name's first letter (flutter-tizen#13)
cb8ee9b Handle key events for tizen webview (flutter-tizen#11)
44987de [webview_flutter] Add webview_flutter initial package (flutter-tizen#7)

Co-authored-by: MuHong Byun <mh.byun@samsung.com>
Co-authored-by: Seungsoo Lee <seungsoo47.lee@samsung.com>
@seungsoo47 seungsoo47 force-pushed the webview_tizen branch 2 times, most recently from 4ed2ec6 to 4e01af6 Compare April 1, 2021 06:08
Copy link
Member

@swift-kim swift-kim left a comment

Choose a reason for hiding this comment

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

Please avoid using force-push multiple times as it makes difficult to track code changes. 😢

Theses are just markers for revisiting the original source code later:

  • TizenWebView was copied and modified from AndroidWebView in package:webview_flutter/src/webview_android.dart.
    • TizenView was copied and modified from AndroidView in package:flutter/src/widgets/platform_view.dart.
      • _TizenWebViewState was copied and modified from _AndroidViewState in the same file.
        • _TizenPlatformTextureView was copied from _AndroidPlatformView in the same file.
          • RenderTizenView was copied and modified from RenderAndroidView in package:flutter/src/rendering/platform_view.dart
            • The enums PlatformViewHitTestBehavior and _PlatformViewState are from the same file.
            • _PlatformViewGestureMixin and _PlatformViewGestureRecognizer were copied from the same file.
              • _factoryTypesSetEquals and _factoriesTypeSet are from the same file.
        • TizenViewController was copied and modified from AndroidViewController and TextureAndroidViewController in package:flutter/src/services/platform_views.dart.
          • The enum _TizenViewState is the equivalent of _AndroidViewState in the same file.
        • PlatformViewsServiceTizen was copied and modified from PlatformViewsService in package:flutter/src/services/platform_views.dart.

Please correct me if anything is wrong.

packages/webview_flutter/README.md Outdated Show resolved Hide resolved
packages/webview_flutter/example/tizen/tizen-manifest.xml Outdated Show resolved Hide resolved
packages/webview_flutter/lib/webview_flutter_tizen.dart Outdated Show resolved Hide resolved
packages/webview_flutter/README.md Outdated Show resolved Hide resolved
* Replace tabs to spaces in the manifest file
* Fix broken READEME.md
* Remove unnecessary code
* In accordance with the guideline, I have updated the initialization code.
packages/webview_flutter/tizen/src/webview_factory.cc Outdated Show resolved Hide resolved
packages/webview_flutter/tizen/src/webview.cc Show resolved Hide resolved
}
} else {
throw std::invalid_argument("Unknown WebView setting: " + k);
}
Copy link
Member

Choose a reason for hiding this comment

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

The following two properties were added in webview_flutter:

I don't know if inline media playback is applicable for Tizen, but at least I can see the RegisterOnProgressChangedHandler API in the LWE WebContainer so hasProgressTracking might be supported by webview_flutter_tizen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This plugin is in alpha version. As far as I know, the APIs have been added to the latest version. When our plugin is updated to chromium-based webview, APIs may be also updated.

packages/webview_flutter/tizen/src/webview.cc Show resolved Hide resolved
packages/webview_flutter/tizen/src/webview.cc Show resolved Hide resolved
packages/webview_flutter/tizen/src/webview.cc Outdated Show resolved Hide resolved
* Data files will be created in application's private area.
* Key event including Shift key value will be handled properly.
@HakkyuKim
Copy link
Contributor

HakkyuKim commented Apr 6, 2021

I believe libraries that a plugin depends on (liblightweight-web-engine.flutter.so in this case) should not be manually copied from the plugin directory into a project directory by the developer. Instead, they should automatically be copied by the flutter-tizen tool into a build output directory directly (this can be done in TizenPlugins of tizen_build_target.dart). I or @HakkyuKim will prepare a fix for this if you agree.

Work in progress: flutter-tizen/support-binary-dependencies-in-plugin

@HakkyuKim
Copy link
Contributor

I believe libraries that a plugin depends on (liblightweight-web-engine.flutter.so in this case) should not be manually copied from the plugin directory into a project directory by the developer. Instead, they should automatically be copied by the flutter-tizen tool into a build output directory directly (this can be done in TizenPlugins of tizen_build_target.dart). I or @HakkyuKim will prepare a fix for this if you agree.

Work in progress: flutter-tizen/support-binary-dependencies-in-plugin

The feature is merged to master, the tool will now put dependent binaries in tpks. You need to set the project_def.prop file as follows:

...
USER_LIB_DIRS = lib/${BUILD_ARCH}
USER_LIBS = lightweight-web-engine.flutter
...

and place liblightweight-web-engine.flutter.so in tizen/lib/armel.

Future plugin projects created by the tool will have the following comments written in the project_def.prop file:

# To provide libraries for your plugin, you must put them in specific
# directories that match their supporting cpu architecture type.
# Due to compatibility issue with Tizen CLI, directory names must be armel for arm 
# and i586 for x86. 

* Because the latest flutter-tizen supports automatic copy function of plugin-dependent libraries,
  I updated a prof file and removed a library using in this plugin.
@seungsoo47
Copy link
Contributor Author

seungsoo47 commented Apr 7, 2021

@HakkyuKim Thank you. I updated it in accordance with your guidance and I checked that the example worked fine.

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.

5 participants