Skip to content

Conversation

@bwikbs
Copy link
Member

@bwikbs bwikbs commented May 26, 2022

Contributes to #389.

@bwikbs bwikbs force-pushed the issue_389_webview branch 2 times, most recently from 5602abd to 2566fe8 Compare May 26, 2022 06:01
@bwikbs
Copy link
Member Author

bwikbs commented May 26, 2022

These files are not formatted correctly (see diff below):
  webview_flutter/lib/src/platform_view.dart
  webview_flutter/lib/src/platform_view_tizen.dart

To fix run "pub global activate flutter_plugin_tools && pub global run flutter_plugin_tools format" or copy-paste this command into your terminal:
patch -p1 <<DONE
diff --git a/packages/webview_flutter/lib/src/platform_view.dart b/packages/webview_flutter/lib/src/platform_view.dart
index 4778918..0d914a0 100644
--- a/packages/webview_flutter/lib/src/platform_view.dart
+++ b/packages/webview_flutter/lib/src/platform_view.dart
@@ -149,8 +149,7 @@ mixin _PlatformViewGestureMixin on RenderBox implements MouseTrackerAnnotation {
   set hitTestBehavior(PlatformViewHitTestBehavior value) {
     if (value != _hitTestBehavior) {
       _hitTestBehavior = value;
-      if (owner != null)
-        markNeedsPaint();
+      if (owner != null) markNeedsPaint();
     }
   }
 
diff --git a/packages/webview_flutter/lib/src/platform_view_tizen.dart b/packages/webview_flutter/lib/src/platform_view_tizen.dart
index 7771[48](https://github.com/flutter-tizen/plugins/runs/6604969616?check_suite_focus=true#step:5:49)e..e69a0e6 100644
--- a/packages/webview_flutter/lib/src/platform_view_tizen.dart
+++ b/packages/webview_flutter/lib/src/platform_view_tizen.dart
@@ -244,11 +244,9 @@ class TizenViewController extends PlatformViewController {
   }
 
   Future<void> setOffset(Offset off) async {
-    if (off == _off)
-      return;
+    if (off == _off) return;
 
-    if (_state != _TizenViewState.created)
-      return;
+    if (_state != _TizenViewState.created) return;
 
     _off = off;
 
@@ -263,8 +261,7 @@ class TizenViewController extends PlatformViewController {
   }
 
   Future<void> _sendCreateMessage({Size? size}) async {
-    if (size == null) 
-      return;
+    if (size == null) return;
 
     assert(!size.isEmpty,
         'trying to create $TizenViewController without setting a valid size.');
@@ -[53](https://github.com/flutter-tizen/plugins/runs/6604969616?check_suite_focus=true#step:5:54)8,8 +535,7 @@ class RenderTizenView extends RenderBox with _PlatformViewGestureMixin {
   }
 
   Future<void> _sizePlatformView() async {
-    if (_state == _PlatformViewState.resizing || size.isEmpty) 
-      return;
+    if (_state == _PlatformViewState.resizing || size.isEmpty) return;
 
     _state = _PlatformViewState.resizing;
     markNeedsPaint();
@@ -592,8 +[58](https://github.com/flutter-tizen/plugins/runs/6604969616?check_suite_focus=true#step:5:59)8,7 @@ class RenderTizenView extends RenderBox with _PlatformViewGestureMixin {
   }
 
   void _paintTexture(PaintingContext context, Offset offset) {
-    if (_currentTextureSize == null) 
-      return;
+    if (_currentTextureSize == null) return;
 
     context.addLayer(TextureLayer(
       rect: offset & _currentTextureSize!,

DONE
Error: Process completed with exit code 1.
bwikbs@nut:~/workspace/flutter-tizen/plugins$ dart analyze --fatal-infos packages/webview_flutter/
Analyzing webview_flutter...           4.4s

   info • lib/src/platform_view.dart:152:26 • Separate the control structure expression from its statement. • always_put_control_body_on_new_line
   info • lib/src/platform_view_tizen.dart:247:22 • Separate the control structure expression from its statement. • always_put_control_body_on_new_line
   info • lib/src/platform_view_tizen.dart:249:44 • Separate the control structure expression from its statement. • always_put_control_body_on_new_line
   info • lib/src/platform_view_tizen.dart:264:23 • Separate the control structure expression from its statement. • always_put_control_body_on_new_line
   info • lib/src/platform_view_tizen.dart:538:64 • Separate the control structure expression from its statement. • always_put_control_body_on_new_line
   info • lib/src/platform_view_tizen.dart:591:38 • Separate the control structure expression from its statement. • always_put_control_body_on_new_line

@HakkyuKim @swift-kim
IMO, They have different opinions between clang formatter and dart analyze... Is that so?

@HakkyuKim
Copy link
Contributor

@bwikbs
A fix would be adding curly braces to all single line if statements as done in flutter/flutter@e71eb18.

@bwikbs
Copy link
Member Author

bwikbs commented May 26, 2022

@bwikbs A fix would be adding curly braces to all single line if statements as done in flutter/flutter@e71eb18.

Thanks! I'm just curious, is this only happening to us?

@HakkyuKim
Copy link
Contributor

@bwikbs
I'm not sure if flutter/flutter repo runs format or analyze command in CI, but if it does, it will have the same problem. I suspect that's why they submitted that "adding curly braces" fix.

@bwikbs
Copy link
Member Author

bwikbs commented May 26, 2022

@HakkyuKim
Thanks for the kind explanation. I thought we did the same with upstream ci!
Now I understand!

@bwikbs bwikbs force-pushed the issue_389_webview branch from 2566fe8 to 388c1c7 Compare May 27, 2022 01:41
Signed-off-by: MuHong Byun <mh.byun@samsung.com>
@bwikbs bwikbs force-pushed the issue_389_webview branch from 388c1c7 to 8030e29 Compare May 27, 2022 02:26
@bwikbs bwikbs marked this pull request as ready for review May 27, 2022 02:27
@swift-kim
Copy link
Member

Were you able to run integration test? It's failing for some unknown reason (state error) in my case.

$ flutter-tizen test integration_test
...
00:54 +0 -1: loading /tmp/flutter_tools.WYUAKN/MPIMSV/webview_flutter_test.dart [E]
  Failed to load "/tmp/flutter_tools.WYUAKN/MPIMSV/webview_flutter_test.dart": Bad state: Can't call test() once tests have begun running.
  package:test_api                                                                                         Declarer.test
  package:flutter_test/src/test_compat.dart 166:13                                                         test
  package:flutter_test/src/widget_tester.dart 153:5                                                        testWidgets
  home/swift/Git/plugins/packages/webview_flutter/example/integration_test/webview_flutter_test.dart 47:3  main

00:54 +0 -1: (tearDownAll) - did not complete [E]
00:54 +0 -1: Some tests failed.

When I tried with the drive command, the test ran but failed for another reason (exceptions).

$ flutter-tizen drive --target integration_test/webview_flutter_test.dart --driver test_driver/integration_test.dart
...
[I]     flutter: 00:03 +1 ~2: loadUrl with headers
[I] flutter: ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
[I] flutter: The following TestFailure was thrown running a test:
[I] flutter: Expected: true
[I] flutter:   Actual: <false>
[I] flutter:
[I] flutter: When the exception was thrown, this was the stack:
[I] flutter: #4      main.<anonymous closure> (file:///home/swift/Git/plugins/packages/webview_flutter/example/integration_test/webview_flutter_test.dart:149:5)
[I] flutter: <asynchronous suspension>
flutter: #5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:170:15)
[I] flutter: <asynchronous suspension>
[I] flutter: #6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:833:5)
[I] flutter: <asynchronous suspension>
[I] flutter:
[I] flutter: This was caught by the test expectation on the following line:
[I] flutter:   file:///home/swift/Git/plugins/packages/webview_flutter/example/integration_test/webview_flutter_test.dart line 149
[I] flutter: The test description was:
[I] flutter:   loadUrl with headers
[I] flutter: ════════════════════════════════════════════════════════════════════════════════════════════════════
[I] flutter: 00:05 +1 ~2: loadUrl with headers [E]
[I] flutter:   Test failed. See exception logs above.
[I]   The test description was: loadUrl with headers
[I] flutter:
[I] flutter: 00:05 +1 ~2 -1: JavascriptChannel
[I] flutter: 00:06 +2 ~2 -1: resize webview
[I]          flutter: 00:12 +2 ~2 -1: evaluateJavascript
[I] flutter: ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
[I] flutter: The following assertion was thrown running a test (but after the test had completed):
[I] flutter: 'package:webview_flutter_tizen/src/platform_view_tizen.dart': Failed assertion: line 240 pos 12:
[I] flutter: 'meta != null': is not true.
[I] flutter:
[I] flutter: When the exception was thrown, this was the stack:
[I] flutter: #2      TizenViewController.setSize (package:webview_flutter_tizen/src/platform_view_tizen.dart:240:12)
[I] flutter: <asynchronous suspension>
[I] flutter: #3      RenderTizenView._sizePlatformView (package:webview_flutter_tizen/src/platform_view_tizen.dart:558:31)
[I] flutter: <asynchronous suspension>
flutter: (elided 2 frames from class _AssertionError)
...

@swift-kim
Copy link
Member

We shouldn't need a Tizen-specific implementation of CookieManager.

You may include this commit in this PR: swift-kim@61f6328

This is a breaking change so you will need to increase the version of the package to 0.5.0.

@bwikbs
Copy link
Member Author

bwikbs commented May 30, 2022

 flutter-tizen test integration_test

I forgot this! Thank you!

swift-kim and others added 2 commits May 31, 2022 11:00
* Update RenderTizenView
* Update change log
* Revert unintended changes
* Update version

Signed-off-by: MuHong Byun <mh.byun@samsung.com>
@bwikbs bwikbs force-pushed the issue_389_webview branch from f8c0638 to 69fc663 Compare May 31, 2022 05:13
@bwikbs
Copy link
Member Author

bwikbs commented Jun 2, 2022

integration_test has a lot of TC which can't pass right now, so I will remove most of it.
BTW, It is executed only with the drive command and not with the test command. I have a speculation this is related to running a test http server, but I don't understand why.

@swift-kim
Copy link
Member

Hmm. The flutter-tizen test integration_test command worked properly before this change, but it's somehow broken now. I don't know why either.

* Removed TC that cannot be performed at this moment
* Minor changes for Tizen webview

Signed-off-by: MuHong Byun <mh.byun@samsung.com>
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.

Are the removed tests not relevant for Tizen?

@bwikbs
Copy link
Member Author

bwikbs commented Jun 3, 2022

Are the removed tests not relevant for Tizen?

To be more precise, some of them are irrelevant and some are currently not passing. 😢
In the near future, I'll pass them one by one.

@bwikbs bwikbs added the no merge This PR is not ready for merge yet label Jun 3, 2022
@bwikbs
Copy link
Member Author

bwikbs commented Jun 3, 2022

lwe binary updates should be included.

* f0ca15ee41d2fc96b59fd57b63b6c32cf6c1906b

Signed-off-by: MuHong Byun <mh.byun@samsung.com>
@bwikbs bwikbs removed the no merge This PR is not ready for merge yet label Jun 7, 2022
bwikbs and others added 2 commits June 7, 2022 14:52
Signed-off-by: MuHong Byun <mh.byun@samsung.com>
@swift-kim swift-kim merged commit 6fe243c into flutter-tizen:master Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants