Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[webview_flutter_wkwebview] Return an NSNumber that represents a bool #5968

Merged
fluttergithubbot merged 4 commits into from
Jun 15, 2022
Merged

[webview_flutter_wkwebview] Return an NSNumber that represents a bool #5968

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
313ad34
Verify a bool value is returned
bparrishMines Jun 13, 2022
c6e4bbb
Merge branch 'main' of github.com:flutter/plugins into boolvnum
bparrishMines Jun 14, 2022
b3f70c6
improve comment
bparrishMines Jun 14, 2022
6d26140
switch sides
bparrishMines Jun 15, 2022
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
2 changes: 2 additions & 0 deletions ...utter/webview_flutter_wkwebview/example/ios/RunnerTests/FWFWebsiteDataStoreHostApiTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ - (void)testRemoveDataOfTypes {
blockError = error;
}];
XCTAssertEqualObjects(returnValue, @YES);
// Asserts whether the NSNumber value represents a BOOL.
XCTAssertEqual(CFBooleanGetTypeID(), CFGetTypeID((__bridge CFTypeRef)(returnValue)));
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Jun 14, 2022

Choose a reason for hiding this comment

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

Compare to CFBooleanGetTypeID()?

Copy link
Contributor Author

@bparrishMines bparrishMines Jun 14, 2022

Choose a reason for hiding this comment

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

Yea, this is essentially code taken from https://github.com/flutter/engine/blob/main/shell/platform/darwin/common/framework/Source/FlutterStandardCodec.mm#L278.

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Jun 14, 2022

Choose a reason for hiding this comment

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

Oh, I just wasn't reading carefully and thought the right hand side was the expected value and that you were extracting the type from a local variable. I was suggesting using CFBooleanGetTypeID() as the expected value instead which is already what you are doing. Oops!

(It is confusing that this test is inconsistent about whether you put the expected value first or second though. The line right above this does the opposite.)

Copy link
Contributor Author

@bparrishMines bparrishMines Jun 15, 2022

Choose a reason for hiding this comment

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

Ah, you're right. I switched the order

XCTAssertNil(blockError);
}
@end
2 changes: 1 addition & 1 deletion packages/webview_flutter/webview_flutter_wkwebview/ios/Classes/FWFWebsiteDataStoreHostApi.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ - (void)createDefaultDataStoreWithIdentifier:(nonnull NSNumber *)identifier
modifiedSince:[NSDate dateWithTimeIntervalSince1970:
modificationTimeInSecondsSinceEpoch.doubleValue]
completionHandler:^{
completion(@(records.count > 0), nil);
completion([NSNumber numberWithBool:(records.count > 0)], nil);
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Jun 14, 2022

Choose a reason for hiding this comment

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

Wow, that's a surprising foot-gun. TIL.

What type was it turning into exactly? Int?

Copy link
Contributor Author

@bparrishMines bparrishMines Jun 14, 2022

Choose a reason for hiding this comment

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

Yea, it was returning an int for some reason.

}];
}];
}
Expand Down