Bug 239591 - REGRESSION (r281791): [iOS] WKWebView cannot load local .log file
Summary: REGRESSION (r281791): [iOS] WKWebView cannot load local .log file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: Safari 15
Hardware: iPhone / iPad iOS 15
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on: 229414
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-20 23:25 PDT by Levin Li
Modified: 2022-05-13 16:13 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.20 KB, patch)
2022-05-11 16:03 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (2.20 KB, patch)
2022-05-12 13:22 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (1.87 KB, patch)
2022-05-13 15:03 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Levin Li 2022-04-20 23:25:34 PDT
On iOS 15.4, the following code loading a local .log file (works on iOS 15.2.1) stops working.

```
let url = Bundle.main.url(forResource: "ABC", withExtension: "log")!
webView.loadFileURL(url, allowingReadAccessTo: url)
```

Some logs:
```
[Process] 0x7fa558809220 - [pageProxyID=6, webPageID=7, PID=81527] WebPageProxy::didFailProvisionalLoadForFrame: frameID=3, domain=WebKitErrorDomain, code=102, isMainFrame=1
```

Error object in func webView(_ webView: WKWebView, didFailProvisionalNavigation navigation: WKNavigation!, withError error: Error)
```
Error Domain=WebKitErrorDomain Code=102 "Frame load interrupted" UserInfo={_WKRecoveryAttempterErrorKey=<WKReloadFrameErrorRecoveryAttempter: 0x6000000a2120>, NSErrorFailingURLStringKey=LOG_FILE_PATH, NSErrorFailingURLKey=LOG_FILE_PATH, NSLocalizedDescription=Frame load interrupted}

```
Comment 1 Radar WebKit Bug Importer 2022-04-27 23:26:13 PDT
<rdar://problem/92442408>
Comment 2 Smoley 2022-04-28 08:23:31 PDT
Thanks for filing, can you please submit a sysdiagnose via http://feedbackassistant.apple.com/ and mention this report (Bug 239591)?

When filing that report please capture a sysdiagnose immediately after the issue reproduces with the web view in the foreground. Additionally, please note the exact time (ex: 1/1/19 12:51PM) and URL so we can investigate further. Thanks again!

macOS Sysdiagnose documentation: https://developer.apple.com/services-account/download?path=/OS_X/OS_X_Logs/sysdiagnose_Logging_Instructions.pdf
Comment 3 Levin Li 2022-04-28 19:11:48 PDT
(In reply to Smoley from comment #2)
> Thanks for filing, can you please submit a sysdiagnose via
> http://feedbackassistant.apple.com/ and mention this report (Bug 239591)?
> 
> When filing that report please capture a sysdiagnose immediately after the
> issue reproduces with the web view in the foreground. Additionally, please
> note the exact time (ex: 1/1/19 12:51PM) and URL so we can investigate
> further. Thanks again!
> 
> macOS Sysdiagnose documentation:
> https://developer.apple.com/services-account/download?path=/OS_X/OS_X_Logs/
> sysdiagnose_Logging_Instructions.pdf


I've filed a feedback with FB10000223 and attached an Xcode project for reproducing and a sysdiag
Comment 4 Smoley 2022-04-29 07:52:35 PDT
Thanks Levin I've added the attachments to the original report and will route accordingly.
Comment 5 Brent Fulgham 2022-05-11 15:41:09 PDT
It looks like this regressed in the iOS-specific changes for Bug 229414.
Comment 6 Brent Fulgham 2022-05-11 15:47:08 PDT
This use case was previously handled in the Quicklook code path (even though QL isn't actually needed for this).
Comment 7 Brent Fulgham 2022-05-11 16:03:26 PDT
Created attachment 459187 [details]
Patch
Comment 8 Geoffrey Garen 2022-05-11 16:15:38 PDT
Comment on attachment 459187 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=459187&action=review

r=me

Does something specifically prevent us from adding a regression test that loads a .log file?

> Source/WebCore/platform/network/ios/WebCoreURLResponseIOS.mm:72
> +    } else
>  #endif // USE(QUICK_LOOK)
> +    if (!type)
> +        CFURLResponseSetMIMEType(response, CFSTR("application/octet-stream"));
>  }

Super-hard to read an else-if that spans an #ifdef. Can the previous block use an early return instead?
Comment 9 Brent Fulgham 2022-05-11 17:06:29 PDT
Comment on attachment 459187 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=459187&action=review

> Source/WebCore/platform/network/ios/WebCoreURLResponseIOS.mm:-53
> -        return;

@Darin: It's surprising to me that our iOS code path only ever dealt with the amusingly-named 'mjs' file extension, while our macOS code deals with a myriad of things. Before your refactoring, we would fall through to the QuickLook case which seems like it would generally hand off to the equivalent of your new 'preferredMIMETypeForFileExtensionFromUTType' method, and return a valid MIME type.

I wonder if we should just always check 'preferredMIMETypeForFileExtensionFromUTType' if we have a valid file URL, rather than depending on the main resource load + QuickLook case handling them.
Comment 10 Brent Fulgham 2022-05-11 17:07:07 PDT
(In reply to Geoffrey Garen from comment #8)
> Comment on attachment 459187 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=459187&action=review
> 
> r=me
> 
> Does something specifically prevent us from adding a regression test that
> loads a .log file?
> 
> > Source/WebCore/platform/network/ios/WebCoreURLResponseIOS.mm:72
> > +    } else
> >  #endif // USE(QUICK_LOOK)
> > +    if (!type)
> > +        CFURLResponseSetMIMEType(response, CFSTR("application/octet-stream"));
> >  }
> 
> Super-hard to read an else-if that spans an #ifdef. Can the previous block
> use an early return instead?

Oh, gosh -- yes. I should have done that instead. I'll fix before landing, though I'm going to wait until Darin has had a look since he may have other ideas for how to address this.
Comment 11 Darin Adler 2022-05-11 17:17:22 PDT
Comment on attachment 459187 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=459187&action=review

>> Source/WebCore/platform/network/ios/WebCoreURLResponseIOS.mm:-53
>> -        return;
> 
> @Darin: It's surprising to me that our iOS code path only ever dealt with the amusingly-named 'mjs' file extension, while our macOS code deals with a myriad of things. Before your refactoring, we would fall through to the QuickLook case which seems like it would generally hand off to the equivalent of your new 'preferredMIMETypeForFileExtensionFromUTType' method, and return a valid MIME type.
> 
> I wonder if we should just always check 'preferredMIMETypeForFileExtensionFromUTType' if we have a valid file URL, rather than depending on the main resource load + QuickLook case handling them.

I did make a mistake in my refactoring. I assumed that shouldUseQuickLookForMIMEType would return false if passed null for a MIME type, and clearly it returns true for that case. Oops!

Your change looks fine.
Comment 12 Brent Fulgham 2022-05-12 13:22:49 PDT
Created attachment 459245 [details]
Patch for landing
Comment 13 EWS 2022-05-12 13:59:31 PDT
Committed r294118 (250499@main): <https://commits.webkit.org/250499@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459245 [details].
Comment 14 Levin Li 2022-05-12 21:17:37 PDT
Thanks for the quick fix! Do you know in which iOS version will this fix be included?
Comment 15 Darin Adler 2022-05-13 13:50:13 PDT
Brent, please consider my comment: https://github.com/WebKit/WebKit/commit/5883fa195996d3faa26e6a8453505a1169548719#﷒0﷓

Levin, Apple folks generally can’t announce which fixes go into which iOS versions. I understand how useful it would be for you to have that information!
Comment 16 Brent Fulgham 2022-05-13 15:03:39 PDT
Reopening to add a follow-up fix.
Comment 17 Brent Fulgham 2022-05-13 15:03:45 PDT
Created attachment 459322 [details]
Patch for landing
Comment 18 EWS 2022-05-13 16:13:26 PDT
Committed r294182 (250549@main): <https://commits.webkit.org/250549@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 459322 [details].