This repository has been archived by the owner on Feb 22, 2023. It is now read-only.
[webview_flutter] Refactored creation of Android WebView for testability. #4178
Merged
fluttergithubbot
merged 7 commits into
flutter:master
from
Baseflow:webview/android_webview_builder
Jul 22, 2021
Merged
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
5d04311
Refactor for testability
mvanbeusekom c026f9d
Test FlutterWebView.createWebView method
mvanbeusekom 9bce993
Fixed formatting
mvanbeusekom da1b1c3
Add missing license header
mvanbeusekom 953189d
Processed PR feedback
mvanbeusekom 2c15ee0
Fixed formatting
mvanbeusekom b2b72a2
Fixed formatting
mvanbeusekom File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
140 changes: 140 additions & 0 deletions
140
...bview_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewBuilder.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package io.flutter.plugins.webviewflutter; | ||
|
|
||
| import android.content.Context; | ||
| import android.view.View; | ||
| import android.webkit.WebChromeClient; | ||
| import android.webkit.WebSettings; | ||
| import android.webkit.WebView; | ||
| import androidx.annotation.NonNull; | ||
|
|
||
| /** Builder used to create {@link android.webkit.WebView} objects. */ | ||
| public class WebViewBuilder { | ||
|
|
||
| /** Factory used to create a new {@link android.webkit.WebView} instance. */ | ||
| static class WebViewFactory { | ||
|
|
||
| /** | ||
| * Creates a new {@link android.webkit.WebView} instance. | ||
| * | ||
| * @param context an Activity Context to access application assets. This value cannot be null. | ||
| * @param usesHybridComposition If {@code false} a {@link InputAwareWebView} instance is | ||
| * returned. | ||
| * @param containerView must be supplied when the {@code useHybridComposition} parameter is set | ||
| * to {@code false}. Used to create an InputConnection on the WebView's dedicated input, or | ||
| * IME, thread (see also {@link InputAwareWebView}) | ||
| * @return A new instance of the {@link android.webkit.WebView} object. | ||
| */ | ||
| static WebView create(Context context, boolean usesHybridComposition, View containerView) { | ||
| return usesHybridComposition | ||
| ? new WebView(context) | ||
| : new InputAwareWebView(context, containerView); | ||
| } | ||
| } | ||
|
|
||
| private final Context context; | ||
| private final View containerView; | ||
|
|
||
| private boolean enableDomStorage; | ||
| private boolean javaScriptCanOpenWindowsAutomatically; | ||
| private boolean supportMultipleWindows; | ||
| private boolean usesHybridComposition; | ||
| private WebChromeClient webChromeClient; | ||
|
|
||
| /** | ||
| * Constructs a new {@link WebViewBuilder} object with a custom implementation of the {@link | ||
| * WebViewFactory} object. | ||
| * | ||
| * @param context an Activity Context to access application assets. This value cannot be null. | ||
| * @param containerView must be supplied when the {@code useHybridComposition} parameter is set to | ||
| * {@code false}. Used to create an InputConnection on the WebView's dedicated input, or IME, | ||
| * thread (see also {@link InputAwareWebView}) | ||
| */ | ||
| WebViewBuilder(@NonNull final Context context, View containerView) { | ||
| this.context = context; | ||
| this.containerView = containerView; | ||
| } | ||
|
|
||
| /** | ||
| * Sets whether the DOM storage API is enabled. The default value is {@code false}. | ||
| * | ||
| * @param flag {@code true} is {@link android.webkit.WebView} should use the DOM storage API. | ||
| * @return This builder. This value cannot be {@code null}. | ||
| */ | ||
| public WebViewBuilder setDomStorageEnabled(boolean flag) { | ||
| this.enableDomStorage = flag; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Sets whether JavaScript is allowed to open windows automatically. This applies to the | ||
| * JavaScript function {@code window.open()}. The default value is {@code false}. | ||
| * | ||
| * @param flag {@code true} if JavaScript is allowed to open windows automatically. | ||
| * @return This builder. This value cannot be {@code null}. | ||
| */ | ||
| public WebViewBuilder setJavaScriptCanOpenWindowsAutomatically(boolean flag) { | ||
| this.javaScriptCanOpenWindowsAutomatically = flag; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Sets whether the {@link WebView} supports multiple windows. If set to {@code true}, {@link | ||
| * WebChromeClient#onCreateWindow} must be implemented by the host application. The default is | ||
| * {@code false}. | ||
| * | ||
| * @param flag {@code true} if multiple windows are supported. | ||
| * @return This builder. This value cannot be {@code null}. | ||
| */ | ||
| public WebViewBuilder setSupportMultipleWindows(boolean flag) { | ||
| this.supportMultipleWindows = flag; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Sets whether the hybrid composition should be used. | ||
| * | ||
| * <p>If set to {@code true} a standard {@link WebView} is created. If set to {@code false} the | ||
| * {@link WebViewBuilder} will create a {@link InputAwareWebView} to workaround issues using the | ||
| * {@link WebView} on Android versions below N. | ||
| * | ||
| * @param flag {@code true} if uses hybrid composition. The default is {@code false}. | ||
| * @return This builder. This value cannot be {@code null} | ||
| */ | ||
| public WebViewBuilder setUsesHybridComposition(boolean flag) { | ||
| this.usesHybridComposition = flag; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the chrome handler. This is an implementation of WebChromeClient for use in handling | ||
| * JavaScript dialogs, favicons, titles, and the progress. This will replace the current handler. | ||
| * | ||
| * @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) { | ||
mvanbeusekom marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| this.webChromeClient = webChromeClient; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Build the {@link android.webkit.WebView} using the current settings. | ||
| * | ||
| * @return The {@link android.webkit.WebView} using the current settings. | ||
| */ | ||
| public WebView build() { | ||
| WebView webView = WebViewFactory.create(context, usesHybridComposition, containerView); | ||
|
|
||
| WebSettings webSettings = webView.getSettings(); | ||
| webSettings.setDomStorageEnabled(enableDomStorage); | ||
| webSettings.setJavaScriptCanOpenWindowsAutomatically(javaScriptCanOpenWindowsAutomatically); | ||
| webSettings.setSupportMultipleWindows(supportMultipleWindows); | ||
| webView.setWebChromeClient(webChromeClient); | ||
|
|
||
| return webView; | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
...w_flutter/android/src/test/java/io/flutter/plugins/webviewflutter/FlutterWebViewTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package io.flutter.plugins.webviewflutter; | ||
|
|
||
| import static org.mockito.ArgumentMatchers.any; | ||
| import static org.mockito.ArgumentMatchers.anyBoolean; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.times; | ||
| import static org.mockito.Mockito.verify; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| import android.webkit.WebChromeClient; | ||
| import android.webkit.WebView; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| public class FlutterWebViewTest { | ||
| private WebChromeClient mockWebChromeClient; | ||
| private WebViewBuilder mockWebViewBuilder; | ||
| private WebView mockWebView; | ||
|
|
||
| @Before | ||
| public void before() { | ||
| mockWebChromeClient = mock(WebChromeClient.class); | ||
| mockWebViewBuilder = mock(WebViewBuilder.class); | ||
| mockWebView = mock(WebView.class); | ||
|
|
||
| when(mockWebViewBuilder.setDomStorageEnabled(anyBoolean())).thenReturn(mockWebViewBuilder); | ||
| when(mockWebViewBuilder.setJavaScriptCanOpenWindowsAutomatically(anyBoolean())) | ||
| .thenReturn(mockWebViewBuilder); | ||
| when(mockWebViewBuilder.setSupportMultipleWindows(anyBoolean())).thenReturn(mockWebViewBuilder); | ||
| when(mockWebViewBuilder.setUsesHybridComposition(anyBoolean())).thenReturn(mockWebViewBuilder); | ||
| when(mockWebViewBuilder.setWebChromeClient(any(WebChromeClient.class))) | ||
| .thenReturn(mockWebViewBuilder); | ||
|
|
||
| when(mockWebViewBuilder.build()).thenReturn(mockWebView); | ||
| } | ||
|
|
||
| @Test | ||
| public void createWebView_should_create_webview_with_default_configuration() { | ||
| FlutterWebView.createWebView( | ||
| mockWebViewBuilder, createParameterMap(false), mockWebChromeClient); | ||
|
|
||
| verify(mockWebViewBuilder, times(1)).setDomStorageEnabled(true); | ||
| verify(mockWebViewBuilder, times(1)).setJavaScriptCanOpenWindowsAutomatically(true); | ||
| verify(mockWebViewBuilder, times(1)).setSupportMultipleWindows(true); | ||
| verify(mockWebViewBuilder, times(1)).setUsesHybridComposition(false); | ||
| verify(mockWebViewBuilder, times(1)).setWebChromeClient(mockWebChromeClient); | ||
| } | ||
|
|
||
| private Map<String, Object> createParameterMap(boolean usesHybridComposition) { | ||
| Map<String, Object> params = new HashMap<>(); | ||
| params.put("usesHybridComposition", usesHybridComposition); | ||
|
|
||
| return params; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 theFlutterWebViewconstructor.There was a problem hiding this comment.
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
FlutterWebViewconstructor. I can make that happen.There was a problem hiding this comment.
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
paramsalso contain a secondMap<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.