Skip to content

Commit

Permalink
WKAttachmentTests: Return BOOL from helper methods that take NSError …
Browse files Browse the repository at this point in the history 
…** argument

https://bugs.webkit.org/show_bug.cgi?id=249198
<rdar://103280901>

Reviewed by Wenson Hsieh.

Change helper methods that take an `NSError **` argument to
return `BOOL`.  This addresses a clang static analyer warning
for the osx.cocoa.NSError checker, and improves the tests since
they all pass `nil` for the `error:` parameter and were not
checking for errors.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(-[_WKAttachment synchronouslySetFileWrapper:newContentType:error:]):
(-[_WKAttachment synchronouslySetData:newContentType:newFilename:error:]):
- Change methods to return BOOL instead of void.
(TestWebKitAPI::TEST):
- Update tests to check return value of these methods since they
  all pass `nil` for the `error:` parameter.

Canonical link: https://commits.webkit.org/257790@main
  • Loading branch information
@ddkilzer
David Kilzer authored and ddkilzer committed Dec 13, 2022
1 parent 23a93ba commit 9303160
Showing 1 changed file with 38 additions and 17 deletions.
55 changes: 38 additions & 17 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
View file
Expand Up @@ -458,7 +458,7 @@ - (NSString *)shortDescription

@implementation _WKAttachment (AttachmentTesting)

- (void)synchronouslySetFileWrapper:(NSFileWrapper *)fileWrapper newContentType:(NSString *)newContentType error:(NSError **)error
- (BOOL)synchronouslySetFileWrapper:(NSFileWrapper *)fileWrapper newContentType:(NSString *)newContentType error:(NSError **)error
{
__block RetainPtr<NSError> resultError;
__block bool done = false;
Expand All @@ -470,11 +470,14 @@ - (void)synchronouslySetFileWrapper:(NSFileWrapper *)fileWrapper newContentType:

TestWebKitAPI::Util::run(&done);

BOOL errorOccurred = !!resultError;
if (error)
*error = resultError.autorelease();

return errorOccurred;
}

- (void)synchronouslySetData:(NSData *)data newContentType:(NSString *)newContentType newFilename:(NSString *)newFilename error:(NSError **)error
- (BOOL)synchronouslySetData:(NSData *)data newContentType:(NSString *)newContentType newFilename:(NSString *)newFilename error:(NSError **)error
{
__block RetainPtr<NSError> resultError;
__block bool done = false;
Expand All @@ -489,8 +492,11 @@ - (void)synchronouslySetData:(NSData *)data newContentType:(NSString *)newConten

TestWebKitAPI::Util::run(&done);

BOOL errorOccurred = !!resultError;
if (error)
*error = resultError.autorelease();

return errorOccurred;
}

- (void)expectRequestedDataToBe:(NSData *)expectedData
Expand Down Expand Up @@ -1019,8 +1025,10 @@ @implementation FileWrapper
{
// Swap the two attachments' file wrappers without creating or destroying attachment elements.
ObserveAttachmentUpdatesForScope observer(webView.get());
[firstAttachment synchronouslySetFileWrapper:folder.get() newContentType:nil error:nil];
[secondAttachment synchronouslySetFileWrapper:file.get() newContentType:nil error:nil];
BOOL errorOccurred = [firstAttachment synchronouslySetFileWrapper:folder.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
errorOccurred = [secondAttachment synchronouslySetFileWrapper:file.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
observer.expectAttachmentUpdates(@[ ], @[ ]);
}

Expand All @@ -1043,7 +1051,8 @@ @implementation FileWrapper
{
RetainPtr<NSData> imageData = testImageData();
ObserveAttachmentUpdatesForScope observer(webView.get());
[attachment synchronouslySetData:imageData.get() newContentType:@"image/png" newFilename:@"icon.png" error:nil];
BOOL errorOccurred = [attachment synchronouslySetData:imageData.get() newContentType:@"image/png" newFilename:@"icon.png" error:nil];
EXPECT_EQ(NO, errorOccurred);
[attachment expectRequestedDataToBe:imageData.get()];
EXPECT_WK_STREQ(@"icon.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
Expand All @@ -1053,7 +1062,8 @@ @implementation FileWrapper
RetainPtr<NSData> textData = [@"Hello world" dataUsingEncoding:NSUTF8StringEncoding];
ObserveAttachmentUpdatesForScope observer(webView.get());
// The new content type should be inferred from the file name.
[attachment synchronouslySetData:textData.get() newContentType:nil newFilename:@"foo.txt" error:nil];
BOOL errorOccurred = [attachment synchronouslySetData:textData.get() newContentType:nil newFilename:@"foo.txt" error:nil];
EXPECT_EQ(NO, errorOccurred);
[attachment expectRequestedDataToBe:textData.get()];
EXPECT_WK_STREQ(@"foo.txt", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
EXPECT_WK_STREQ(@"text/plain", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
Expand All @@ -1062,7 +1072,8 @@ @implementation FileWrapper
{
RetainPtr<NSData> htmlData = testHTMLData();
ObserveAttachmentUpdatesForScope observer(webView.get());
[attachment synchronouslySetData:htmlData.get() newContentType:@"text/html" newFilename:@"bar" error:nil];
BOOL errorOccurred = [attachment synchronouslySetData:htmlData.get() newContentType:@"text/html" newFilename:@"bar" error:nil];
EXPECT_EQ(NO, errorOccurred);
[attachment expectRequestedDataToBe:htmlData.get()];
EXPECT_WK_STREQ(@"bar", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
EXPECT_WK_STREQ(@"text/html", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
Expand Down Expand Up @@ -1324,7 +1335,8 @@ @implementation FileWrapper
EXPECT_FALSE([pastedAttachment isEqual:originalAttachment.get()]);
}
auto updatedData = retainPtr([@"HELLO WORLD" dataUsingEncoding:NSUTF8StringEncoding]);
[originalAttachment synchronouslySetData:updatedData.get() newContentType:nil newFilename:nil error:nil];
BOOL errorOccurred = [originalAttachment synchronouslySetData:updatedData.get() newContentType:nil newFilename:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
[originalAttachment expectRequestedDataToBe:updatedData.get()];
[pastedAttachment expectRequestedDataToBe:originalData.get()];

Expand Down Expand Up @@ -1353,7 +1365,8 @@ @implementation FileWrapper
auto newFileWrapper = adoptNS([[NSFileWrapper alloc] initWithURL:testPDFFileURL() options:0 error:nil]);
{
ObserveAttachmentUpdatesForScope observer(webView.get());
[attachment synchronouslySetFileWrapper:newFileWrapper.get() newContentType:nil error:nil];
BOOL errorOccurred = [attachment synchronouslySetFileWrapper:newFileWrapper.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
observer.expectAttachmentUpdates(@[ ], @[ ]);
}

Expand Down Expand Up @@ -1514,10 +1527,12 @@ @implementation FileWrapper

auto newImage = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testGIFData()]);
[newImage setPreferredFilename:@"foo.gif"];
[attachment synchronouslySetFileWrapper:newImage.get() newContentType:nil error:nil];
BOOL errorOccurred = [attachment synchronouslySetFileWrapper:newImage.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
[webView waitForImageElementSizeToBecome:CGSizeMake(52, 64)];

[attachment synchronouslySetFileWrapper:originalImageData.get() newContentType:@"image/png" error:nil];
errorOccurred = [attachment synchronouslySetFileWrapper:originalImageData.get() newContentType:@"image/png" error:nil];
EXPECT_EQ(NO, errorOccurred);
[webView waitForImageElementSizeToBecome:CGSizeMake(215, 174)];
}

Expand All @@ -1537,13 +1552,16 @@ @implementation FileWrapper
auto secondImage = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testGIFData()]);
[secondImage setPreferredFilename:@"foo.gif"];

[attachment synchronouslySetFileWrapper:firstImage.get() newContentType:@"image/png" error:nil];
BOOL errorOccurred = [attachment synchronouslySetFileWrapper:firstImage.get() newContentType:@"image/png" error:nil];
EXPECT_EQ(NO, errorOccurred);
[webView waitForImageElementSizeToBecome:CGSizeMake(215, 174)];

[attachment synchronouslySetFileWrapper:secondImage.get() newContentType:(__bridge NSString *)kUTTypeGIF error:nil];
errorOccurred = [attachment synchronouslySetFileWrapper:secondImage.get() newContentType:(__bridge NSString *)kUTTypeGIF error:nil];
EXPECT_EQ(NO, errorOccurred);
[webView waitForImageElementSizeToBecome:CGSizeMake(52, 64)];

[attachment synchronouslySetFileWrapper:firstImage.get() newContentType:nil error:nil];
errorOccurred = [attachment synchronouslySetFileWrapper:firstImage.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
[webView waitForImageElementSizeToBecome:CGSizeMake(215, 174)];
}

Expand All @@ -1555,7 +1573,8 @@ @implementation FileWrapper
NSString *identifier = [webView stringByEvaluatingJavaScript:@"image = document.createElement('img'); HTMLAttachmentElement.getAttachmentIdentifier(image)"];
auto image = adoptNS([[NSFileWrapper alloc] initWithURL:testImageFileURL() options:0 error:nil]);
auto attachment = retainPtr([webView _attachmentForIdentifier:identifier]);
[attachment synchronouslySetFileWrapper:image.get() newContentType:nil error:nil];
BOOL errorOccurred = [attachment synchronouslySetFileWrapper:image.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
EXPECT_FALSE([attachment isConnected]);
observer.expectAttachmentUpdates(@[ ], @[ ]);

Expand All @@ -1566,7 +1585,8 @@ @implementation FileWrapper

auto newImage = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testGIFData()]);
[newImage setPreferredFilename:@"test.gif"];
[attachment synchronouslySetFileWrapper:newImage.get() newContentType:nil error:nil];
errorOccurred = [attachment synchronouslySetFileWrapper:newImage.get() newContentType:nil error:nil];
EXPECT_EQ(NO, errorOccurred);
[webView waitForImageElementSizeToBecome:CGSizeMake(52, 64)];
}

Expand Down Expand Up @@ -1652,7 +1672,8 @@ @implementation FileWrapper
[sourceView _synchronouslyExecuteEditCommand:@"InsertHTML" argument:@"<img></img>"];

auto attachment = [sourceView _attachmentForIdentifier:[sourceView ensureAttachmentForImageElement]];
[attachment synchronouslySetFileWrapper:image.get() newContentType:@"image/png" error:nil];
BOOL errorOccurred = [attachment synchronouslySetFileWrapper:image.get() newContentType:@"image/png" error:nil];
EXPECT_EQ(NO, errorOccurred);
[sourceView waitForImageElementSizeToBecome:CGSizeMake(215, 174)];

EXPECT_WK_STREQ("icon.png", [sourceView valueOfAttribute:@"alt" forQuerySelector:@"img"]);
Expand Down

0 comments on commit 9303160

Please sign in to comment.