Skip to content

[webview_flutter_wkwebview] Fixes inspectable compile-time error and crash from equal NSURLs #4340

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Jul 3, 2023
Merged

[webview_flutter_wkwebview] Fixes inspectable compile-time error and crash from equal NSURLs #4340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6f59765
add some fixes
bparrishMines Jun 28, 2023
f8a8879
fix versions
bparrishMines Jun 28, 2023
ad56919
fix changelog
bparrishMines Jun 28, 2023
2b003b3
maybe version bump will fix?
bparrishMines Jun 28, 2023
e985799
set min ios version to 11
bparrishMines Jun 28, 2023
21bbe26
dont check tvos and dont guard test
bparrishMines Jun 29, 2023
173d8a9
Merge branch 'main' of github.com:flutter/packages into fix_wkwebview
bparrishMines Jun 29, 2023
2a91b99
fix error
bparrishMines Jun 29, 2023
561029a
Merge branch 'main' of github.com:flutter/packages into fix_wkwebview
bparrishMines Jul 3, 2023
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
6 changes: 6 additions & 0 deletions packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 3.6.1

* Fixes bug where a native `NSURL` could be removed from an `InstanceManager` if it is equal to an
already present `NSURL`.
* Fixes compile-time error from using `WKWebView.inspectable` on unsupported Xcode versions.

## 3.6.0

* Adds support to enable debugging of web contents on the latest versions of WebKit. See
Expand Down
16 changes: 16 additions & 0 deletions ...bview_flutter/webview_flutter_wkwebview/example/ios/RunnerTests/FWFInstanceManagerTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,20 @@ - (void)testDeallocCallbackIsIgnoredIfNull {
// Tests that this doesn't cause a EXC_BAD_ACCESS crash.
[instanceManager removeInstanceWithIdentifier:0];
}

- (void)testObjectsAreStoredWithPointerHashcode {
FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];

NSURL *url1 = [NSURL URLWithString:@"https://www.flutter.dev"];
NSURL *url2 = [NSURL URLWithString:@"https://www.flutter.dev"];

// Ensure urls are considered equal.
XCTAssertTrue([url1 isEqual:url2]);

[instanceManager addHostCreatedInstance:url1];
[instanceManager addHostCreatedInstance:url2];

XCTAssertNotEqual([instanceManager identifierWithStrongReferenceForInstance:url1],
[instanceManager identifierWithStrongReferenceForInstance:url2]);
}
@end
2 changes: 1 addition & 1 deletion ...ebview_flutter/webview_flutter_wkwebview/example/ios/RunnerTests/FWFWebViewHostApiTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ - (void)testContentInsetsSumAlwaysZeroAfterSetFrame {
XCTAssertTrue(CGRectEqualToRect(webView.frame, CGRectMake(0, 0, 300, 100)));
}

- (void)testSetInspectable API_AVAILABLE(ios(16.4), macos(13.3), tvos(16.4)) {
- (void)testSetInspectable API_AVAILABLE(ios(16.4), macos(13.3)) {
FWFWebView *mockWebView = OCMClassMock([FWFWebView class]);

FWFInstanceManager *instanceManager = [[FWFInstanceManager alloc] init];
Expand Down
14 changes: 11 additions & 3 deletions packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFInstanceManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,17 @@ - (instancetype)init {
_deallocCallback = _deallocCallback ? _deallocCallback : ^(long identifier) {
};
_lockQueue = dispatch_queue_create("FWFInstanceManager", DISPATCH_QUEUE_SERIAL);
_identifiers = [NSMapTable weakToStrongObjectsMapTable];
_weakInstances = [NSMapTable strongToWeakObjectsMapTable];
_strongInstances = [NSMapTable strongToStrongObjectsMapTable];
// Pointer equality is used to prevent collisions of objects that override the `isEqualTo:`
// method.
_identifiers =
[NSMapTable mapTableWithKeyOptions:NSMapTableWeakMemory | NSMapTableObjectPointerPersonality
valueOptions:NSMapTableStrongMemory];
_weakInstances = [NSMapTable
mapTableWithKeyOptions:NSMapTableStrongMemory
valueOptions:NSMapTableWeakMemory | NSMapTableObjectPointerPersonality];
_strongInstances = [NSMapTable
mapTableWithKeyOptions:NSMapTableStrongMemory
valueOptions:NSMapTableStrongMemory | NSMapTableObjectPointerPersonality];
_nextIdentifier = FWFMinHostCreatedIdentifier;
}
return self;
Expand Down
2 changes: 2 additions & 0 deletions packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFWebViewHostApi.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,9 @@ - (void)setInspectableForWebViewWithIdentifier:(NSNumber *)identifier
inspectable:(NSNumber *)inspectable
error:(FlutterError *_Nullable *_Nonnull)error {
if (@available(macOS 13.3, iOS 16.4, tvOS 16.4, *)) {
#if __MAC_OS_X_VERSION_MAX_ALLOWED >= 130300 || __IPHONE_OS_VERSION_MAX_ALLOWED >= 160400
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Jul 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, looking at this again: do @available checks evaluate to false automatically when the compile SDK is lower than the available version? I'm not sure they do, and if they don't then this would, if compiled with an SDK < 16.4 but run on 16.4, silently no-op.

I guess that's not that big a deal for this particular case, but we should investigate before using this pattern again.

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'm not sure they do, and if they don't then this would, if compiled with an SDK < 16.4, but run on 16.4, silently no-op.

Yea, I think you're correct. This could probably use an additional #ifelse that also returns a FlutterError. I didn't quite understand this until after I autosubmitted.

[[self webViewForIdentifier:identifier] setInspectable:inspectable.boolValue];
#endif
} else {
*error = [FlutterError errorWithCode:@"FWFUnsupportedVersionError"
message:@"setInspectable is only supported on versions 16.4+."
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: webview_flutter_wkwebview
description: A Flutter plugin that provides a WebView widget based on Apple's WKWebView control.
repository: https://github.com/flutter/packages/tree/main/packages/webview_flutter/webview_flutter_wkwebview
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22
version: 3.6.0
version: 3.6.1

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down