Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Processed PR feedback
  • Loading branch information
mvanbeusekom committed Jul 22, 2021
commit 953189d4ef36206645bd29fc2e9ce30136970743
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ public void onProgressChanged(WebView view, int progress) {
@SuppressWarnings("unchecked")
FlutterWebView(
final Context context,
BinaryMessenger messenger,
int id,
MethodChannel methodChannel,
Map<String, Object> params,
View containerView) {

Expand All @@ -104,7 +103,8 @@ public void onProgressChanged(WebView view, int progress) {

platformThreadHandler = new Handler(context.getMainLooper());

methodChannel = createMethodChannel(messenger, id, this);
this.methodChannel = methodChannel;
this.methodChannel.setMethodCallHandler(this);

flutterWebViewClient = new FlutterWebViewClient(methodChannel);
Map<String, Object> settings = (Map<String, Object>) params.get("settings");
Expand Down Expand Up @@ -158,7 +158,7 @@ public void onProgressChanged(WebView view, int progress) {
@VisibleForTesting
static WebView createWebView(
WebViewBuilder webViewBuilder, Map<String, Object> params, WebChromeClient webChromeClient) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should pass a Map<String, Object> object here. I think a configuration object is better and we can make a static method to map the Map to a configuration object. Might be a good idea to already pass a configuration object to the FlutterWebView constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but then we should directly pass it into the FlutterWebView constructor. I can make that happen.

Copy link
Contributor Author

@mvanbeusekom mvanbeusekom Jul 22, 2021

Choose a reason for hiding this comment

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

Looking into this more detailed, changing this to a configuration object requires quite a large refactor as the params also contain a second Map<String, Object> collection containing several web settings.

This means we need to change a lot of code that currently validates how to handle different situations (bases on if a key is part of the params or web settings hashmap or not). These changes are not relevant for the problem this PR is trying to solve. So for now I think it would be better to leave it as is and maybe do a separate PR on updating this if needed.

Boolean usesHybridComposition = (Boolean) params.get("usesHybridComposition");
boolean usesHybridComposition = Boolean.TRUE.equals(params.get("usesHybridComposition"));
webViewBuilder
.setUsesHybridComposition(usesHybridComposition)
.setDomStorageEnabled(true) // Always enable DOM storage API.
Expand All @@ -171,30 +171,6 @@ static WebView createWebView(
return webViewBuilder.build();
}

/**
* Creates a {@link MethodChannel} used to handle communication with the Flutter application about
* the {@link FlutterWebView} instance.
*
* <p><strong>Important:</strong> This method is visible for testing purposes only and should
* never be called from outside this class.
*
* @param messenger a {@link BinaryMessenger} to facilitate communication with Flutter.
* @param id an identifier used to create a unique channel name and identify communication with a
* particular {@link FlutterWebView} instance.
* @param handler a {@link MethodCallHandler} responsible for handling messages that arrive from
* the Flutter application.
* @return The {@link MethodChannel} configured using the supplied {@link BinaryMessenger} and
* {@link MethodCallHandler}.
*/
@VisibleForTesting
static MethodChannel createMethodChannel(
BinaryMessenger messenger, int id, MethodCallHandler handler) {
MethodChannel methodChannel = new MethodChannel(messenger, "plugins.flutter.io/webview_" + id);
methodChannel.setMethodCallHandler(handler);

return methodChannel;
}

@Override
public View getView() {
return webView;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.content.Context;
import android.view.View;
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.StandardMessageCodec;
import io.flutter.plugin.platform.PlatformView;
import io.flutter.plugin.platform.PlatformViewFactory;
Expand All @@ -26,6 +27,7 @@ public final class FlutterWebViewFactory extends PlatformViewFactory {
@Override
public PlatformView create(Context context, int id, Object args) {
Map<String, Object> params = (Map<String, Object>) args;
return new FlutterWebView(context, messenger, id, params, containerView);
MethodChannel methodChannel = new MethodChannel(messenger, "plugins.flutter.io/webview_" + id);
return new FlutterWebView(context, methodChannel, params, containerView);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import android.webkit.WebSettings;
import android.webkit.WebView;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

/** Builder used to create {@link android.webkit.WebView} objects. */
public class WebViewBuilder {
Expand Down Expand Up @@ -116,7 +117,7 @@ public WebViewBuilder setUsesHybridComposition(boolean flag) {
* @param webChromeClient an implementation of WebChromeClient This value may be null.
* @return This builder. This value cannot be {@code null}.
*/
public WebViewBuilder setWebChromeClient(WebChromeClient webChromeClient) {
public WebViewBuilder setWebChromeClient(@Nullable WebChromeClient webChromeClient) {
this.webChromeClient = webChromeClient;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,23 @@ public void build_should_set_values() throws IOException {
verify(mockWebSettings).setSupportMultipleWindows(true);
verify(mockWebView).setWebChromeClient(mockWebChromeClient);
}

@Test
public void build_should_use_default_values() throws IOException {
WebSettings mockWebSettings = mock(WebSettings.class);
WebChromeClient mockWebChromeClient = mock(WebChromeClient.class);

when(mockWebView.getSettings()).thenReturn(mockWebSettings);

WebViewBuilder builder =
new WebViewBuilder(mockContext, mockContainerView);

WebView webView = builder.build();

assertNotNull(webView);
verify(mockWebSettings).setDomStorageEnabled(false);
verify(mockWebSettings).setJavaScriptCanOpenWindowsAutomatically(false);
verify(mockWebSettings).setSupportMultipleWindows(false);
verify(mockWebView).setWebChromeClient(null);
}
}