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 17 commits
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
4 changes: 4 additions & 0 deletions packages/webview_flutter/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.3.2

* Added CookieManager to interface with WebView cookies. Currently has the ability to clear cookies.

## 0.3.1

* Added JavaScript channels to facilitate message passing from JavaScript code running inside
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package io.flutter.plugins.webviewflutter;

import android.os.Build;
import android.os.Build.VERSION_CODES;
import android.webkit.CookieManager;
import android.webkit.ValueCallback;
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodCall;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.MethodChannel.MethodCallHandler;
import io.flutter.plugin.common.MethodChannel.Result;

class FlutterCookieManager implements MethodCallHandler {

private FlutterCookieManager() {
// Do not instantiate.
Copy link
Contributor

Choose a reason for hiding this comment

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

explain why

// This class should only be used in context of a BinaryMessenger.
// Use FlutterCookieManager#registerWith instead.
}

static void registerWith(BinaryMessenger messenger) {
MethodChannel methodChannel = new MethodChannel(messenger, "plugins.flutter.io/cookie_manager");
FlutterCookieManager cookieManager = new FlutterCookieManager();
methodChannel.setMethodCallHandler(cookieManager);
}

@Override
public void onMethodCall(MethodCall methodCall, Result result) {
switch (methodCall.method) {
case "clearCookies":
clearCookies(result);
break;
default:
result.notImplemented();
}
}

private static void clearCookies(final Result result) {
CookieManager cookieManager = CookieManager.getInstance();
final boolean hasCookies = cookieManager.hasCookies();
if (Build.VERSION.SDK_INT >= VERSION_CODES.LOLLIPOP) {
cookieManager.removeAllCookies(
new ValueCallback<Boolean>() {
@Override
public void onReceiveValue(Boolean value) {
result.success(hasCookies);
}
});
} else {
cookieManager.removeAllCookie();
result.success(hasCookies);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@

/** WebViewFlutterPlugin */
public class WebViewFlutterPlugin {

/** Plugin registration. */
public static void registerWith(Registrar registrar) {
registrar
.platformViewRegistry()
.registerViewFactory(
"plugins.flutter.io/webview", new WebViewFactory(registrar.messenger()));
FlutterCookieManager.registerWith(registrar.messenger());
}
}
46 changes: 46 additions & 0 deletions packages/webview_flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,14 @@ class WebViewExample extends StatelessWidget {
enum MenuOptions {
showUserAgent,
toast,
clearCookies,
}

class SampleMenu extends StatelessWidget {
SampleMenu(this.controller);

final Future<WebViewController> controller;
final CookieManager cookieManager = CookieManager();

@override
Widget build(BuildContext context) {
Expand All @@ -100,6 +103,9 @@ class SampleMenu extends StatelessWidget {
),
);
break;
case MenuOptions.clearCookies:
_onClearCookies(controller.data, context);
break;
}
},
itemBuilder: (BuildContext context) => <PopupMenuItem<MenuOptions>>[
Expand All @@ -112,6 +118,10 @@ class SampleMenu extends StatelessWidget {
value: MenuOptions.toast,
child: Text('Make a toast'),
),
const PopupMenuItem<MenuOptions>(
value: MenuOptions.clearCookies,
child: Text('Clear cookies'),
),
],
);
},
Expand All @@ -125,6 +135,42 @@ class SampleMenu extends StatelessWidget {
controller.evaluateJavascript(
'Toaster.postMessage("User Agent: " + navigator.userAgent);');
}

void _onClearCookies(
WebViewController controller, BuildContext context) async {
final String cookies =
await controller.evaluateJavascript('document.cookie');
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding a menu entry that just shows a toast with the cookies?
I'd feel more confident to verify that it's working if the button that's showing me the cookies is independent of the button that's deleting the cookies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense.

final bool hadCookies = await cookieManager.clearCookies();
String message = 'There were cookies. Now, they are gone!';
if (!hadCookies) {
message = 'There are no cookies.';
}
Scaffold.of(context).showSnackBar(SnackBar(
content: Column(
mainAxisAlignment: MainAxisAlignment.end,
mainAxisSize: MainAxisSize.min,
children: <Widget>[
Text(message),
const Text('Cookies:'),
_getCookieList(cookies),
],
),
));
}

Widget _getCookieList(String cookies) {
if (cookies == null || cookies == '""') {
return Container();
}
final List<String> cookieList = cookies.split(';');
final Iterable<Text> cookieWidgets =
cookieList.map((String cookie) => Text(cookie));
return Column(
mainAxisAlignment: MainAxisAlignment.end,
mainAxisSize: MainAxisSize.min,
children: cookieWidgets.toList(),
);
}
}

class NavigationControls extends StatelessWidget {
Expand Down
10 changes: 10 additions & 0 deletions packages/webview_flutter/ios/Classes/FLTCookieManager.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#import <Flutter/Flutter.h>
#import <WebKit/WebKit.h>

NS_ASSUME_NONNULL_BEGIN

@interface FLTCookieManager : NSObject <FlutterPlugin>

@end

NS_ASSUME_NONNULL_END
50 changes: 50 additions & 0 deletions packages/webview_flutter/ios/Classes/FLTCookieManager.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#import "FLTCookieManager.h"

@implementation FLTCookieManager {
}

+ (void)registerWithRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
FLTCookieManager *instance = [[FLTCookieManager alloc] init];

FlutterMethodChannel *channel =
[FlutterMethodChannel methodChannelWithName:@"plugins.flutter.io/cookie_manager"
binaryMessenger:[registrar messenger]];
[registrar addMethodCallDelegate:instance channel:channel];
}

- (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result {
if ([[call method] isEqualToString:@"clearCookies"]) {
[self clearCookies:result];
} else {
result(FlutterMethodNotImplemented);
}
}

- (void)clearCookies:(FlutterResult)result {
NSOperatingSystemVersion ios9 = (NSOperatingSystemVersion){9, 0};
if ([[NSProcessInfo processInfo] isOperatingSystemAtLeastVersion:ios9]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (@available(iOS 9.0, *)) {

[self clearCookiesIos9AndLater:result];
} else {
// support for IOS-8 tracked in https://github.com/flutter/flutter/issues/27624.
NSLog(@"Clearing cookies is not supported for Flutter WebViews prior to iOS9.");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/iOS9/iOS 9/

Copy link
Contributor

Choose a reason for hiding this comment

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

ping

}
}

- (void)clearCookiesIos9AndLater:(FlutterResult)result {
NSSet *websiteDataTypes = [NSSet setWithArray:@[ WKWebsiteDataTypeCookies ]];
WKWebsiteDataStore *dataStore = [WKWebsiteDataStore defaultDataStore];

void (^deleteAndNotify)(NSArray<WKWebsiteDataRecord *> *) =
^(NSArray<WKWebsiteDataRecord *> *cookies) {
BOOL hasCookies = cookies.count > 0;
[dataStore removeDataOfTypes:websiteDataTypes
forDataRecords:cookies
completionHandler:^{
result(@(hasCookies));
}];
};

[dataStore fetchDataRecordsOfTypes:websiteDataTypes completionHandler:deleteAndNotify];
}

@end
2 changes: 2 additions & 0 deletions packages/webview_flutter/ios/Classes/WebViewFlutterPlugin.m
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#import "WebViewFlutterPlugin.h"
#import "FLTCookieManager.h"
#import "FlutterWebView.h"

@implementation FLTWebViewFlutterPlugin
Expand All @@ -7,6 +8,7 @@ + (void)registerWithRegistrar:(NSObject<FlutterPluginRegistrar>*)registrar {
FLTWebViewFactory* webviewFactory =
[[FLTWebViewFactory alloc] initWithMessenger:registrar.messenger];
[registrar registerViewFactory:webviewFactory withId:@"plugins.flutter.io/webview"];
[FLTCookieManager registerWithRegistrar:registrar];
}

@end
26 changes: 26 additions & 0 deletions packages/webview_flutter/lib/webview_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,32 @@ class WebViewController {
}
}

/// Manages cookies pertaining to all [WebView]s.
class CookieManager {
factory CookieManager() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a Dartdoc, it should mention that it returns or created the singleton instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not sure if '--' is common in Flutter dartdocs (of if it is grammatically correct), I'd just say:

Returns the singleton cookie manager instance.

The instance is created the first time this factory is called.

if (_instance == null) {
final MethodChannel methodChannel =
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we can set mockMethodCallHandlers for testing, what is the value of providing the channel this way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My original thought was to make this a const field on the class. I wanted to be consistent with Battery plugin, i should maybe look through the test code there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consistency is good to a point, but correctness is better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

curious, does this approach expose a way of injecting the wrong method channel?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not necessary at all

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 that passing in a method channel is not needed for testing, but passing it though this way -- creating it on initialization vs having it static, should be equivalent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Generally making const fields is frowned upon. since the const is already canonicalized we are only adding overhead to the class. Furthermore, removing the initialization from the factory lets us cut down the code a bit with ??=:

factory Foo() {
  return _instance ??= Foo._();
}
Foo._();
static Foo _instance;

const MethodChannel('plugins.flutter.io/cookie_manager');
_instance = CookieManager.private(methodChannel);
}
return _instance;
}

CookieManager.private(this._channel);
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't name a constructor .private, either its private _ or its not

Copy link
Contributor

Choose a reason for hiding this comment

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

the name private has no special meaning.
this should be named _ to make sure it is really private.


static CookieManager _instance;
final MethodChannel _channel;

/// Clears all cookies.
///
/// This is supported for >= IOS 9 and Android api level >= 16.
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to mention Android API level (16 is the minimum level supported by Flutter anyway)

/// returns true if cookies were present before clearing, else false.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: separate the paragraph, start sentence with a capital letter.

Future<bool> clearCookies() => _channel
// ignore: strong_mode_implicit_dynamic_method
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the same comment we have everywhere(minus the typo 😄 ) so it's easy to find all of these at once (and we need a TODO here anyway)

Copy link
Contributor

Choose a reason for hiding this comment

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

keep it TODO(amirh), most likely someone will end up searching for all these TODO(amirh): occurrences at once.

.invokeMethod('clearCookies')
.then<bool>((dynamic result) => result);
}

// Throws an ArgumentError if `url` is not a valid URL string.
void _validateUrlString(String url) {
try {
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: webview_flutter
description: A Flutter plugin that provides a WebView widget on Android and iOS.
version: 0.3.1
version: 0.3.2
author: Flutter Team <flutter-dev@googlegroups.com>
homepage: https://github.com/flutter/plugins/tree/master/packages/webview_flutter

Expand Down
62 changes: 62 additions & 0 deletions packages/webview_flutter/test/webview_flutter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@ void main() {
final _FakePlatformViewsController fakePlatformViewsController =
_FakePlatformViewsController();

final _FakeCookieManager _fakeCookieManager = _FakeCookieManager();

setUpAll(() {
SystemChannels.platform_views.setMockMethodCallHandler(
fakePlatformViewsController.fakePlatformViewsMethodHandler);
SystemChannels.platform
.setMockMethodCallHandler(_fakeCookieManager.onMethodCall);
});

setUp(() {
fakePlatformViewsController.reset();
_fakeCookieManager.reset();
});

testWidgets('Create WebView', (WidgetTester tester) async {
Expand Down Expand Up @@ -356,6 +361,31 @@ void main() {
);
});

testWidgets('Cookies can be cleared once', (WidgetTester tester) async {
await tester.pumpWidget(
const WebView(
initialUrl: 'https://flutter.io',
),
);
final CookieManager cookieManager = CookieManager();
final bool hasCookies = await cookieManager.clearCookies();
expect(hasCookies, true);
});

testWidgets('Second cookie clear does not have cookies',
(WidgetTester tester) async {
await tester.pumpWidget(
const WebView(
initialUrl: 'https://flutter.io',
),
);
final CookieManager cookieManager = CookieManager();
final bool hasCookies = await cookieManager.clearCookies();
expect(hasCookies, true);
final bool hasCookiesSecond = await cookieManager.clearCookies();
expect(hasCookiesSecond, false);
});

testWidgets('Initial JavaScript channels', (WidgetTester tester) async {
await tester.pumpWidget(
WebView(
Expand Down Expand Up @@ -639,3 +669,35 @@ Map<dynamic, dynamic> _decodeParams(Uint8List paramsMessage) {
);
return const StandardMessageCodec().decodeMessage(messageBytes);
}

class _FakeCookieManager {
_FakeCookieManager() {
final MethodChannel channel = const MethodChannel(
'plugins.flutter.io/cookie_manager',
StandardMethodCodec(),
);
channel.setMockMethodCallHandler(onMethodCall);
}

bool hasCookies = true;

Future<bool> onMethodCall(MethodCall call) {
switch (call.method) {
case 'clearCookies':
bool hadCookies = false;
if (hasCookies) {
hadCookies = true;
hasCookies = false;
}
return Future<bool>.sync(() {
return hadCookies;
});
break;
}
return Future<bool>.sync(() {});
}

void reset() {
hasCookies = true;
}
}