CB-11074: Ensure settings from config.xml are taken into consideration

[ephemer]

#7 is also supposed to address this issue, but in my tests it didn't work.

This pull request presents a workaround that requires two WKWebView instances to be inited (only one is kept and used for the app though). This isn't supposed to be merged as is, it's more a proof of concept and designed to provide exposure to the workaround if others users need this fix urgently.

---

@shazron the logic presented in this PR would have been made more complicated via the updateWithInfo: method, which as far as I could see is unused, so I removed it for now. Other than that, what do you think about this as general direction?

As I commented in #7, the best approach for this would be to change CDVViewController.m to pass in the settings directly on init, or to rearrange the check logic there. That is a longer-term solution. In the meantime, maybe we can get by with the approach presented here.

[shazron]

I also commented on #7.

To solve this problem, instead of having to instantiate WKWebView twice here, I think we can safely move all initializing code from the initWithFrame, to pluginInitialize:

cordova-plugin-wkwebview-engine/src/ios/CDVWKWebViewEngine.m

Lines 50 to 64 in 33a7517

	self.uiDelegate = [[CDVWKWebViewUIDelegate alloc] initWithTitle:[[NSBundle mainBundle] objectForInfoDictionaryKey:@"CFBundleDisplayName"]];
	WKUserContentController* userContentController = [[WKUserContentController alloc] init];
	[userContentController addScriptMessageHandler:self name:CDV_BRIDGE_NAME];
	WKWebViewConfiguration* configuration = [[WKWebViewConfiguration alloc] init];
	configuration.userContentController = userContentController;
	WKWebView* wkWebView = [[WKWebView alloc] initWithFrame:frame configuration:configuration];
	wkWebView.UIDelegate = self.uiDelegate;
	self.engineWebView = wkWebView;
	NSLog(@"Using WKWebView");

-- all we need to do is save the CGRect for the frame for later. At this point, with this approach, the WKWebView will have been instantiated for this return call: https://github.com/apache/cordova-ios/blob/e51eda0f3b1e68d210182c3bbd3d9022c1ffe8ec/CordovaLib/Classes/Public/CDVViewController.m#L426

As I mentioned in #7, we still need updateWithInfo.

The rest of your approach is sound.

[ephemer]

@shazron in that case we'd have to remove the configuration code from updateWithInfo because we've already established that it doesn't do anything anyway: it'd be very frustrating for a new developer to see that code and wonder why it does nothing at all. As far as I can see it will never work, wkWebView.configuration is listed as read-only in Apple's docs.

If we can do without the configuration part of updateWithInfo we wouldn't have to worry about having to reinitialize the webview at an arbitrary point in time (which would involve reloading the current URL and a whole bunch of other stuff that I'd rather not have to worry about at all).

I'd also rename updateWithInfo to updateDelegates or similar unless it breaks something else.

---

If we just inited the WKWebView once in pluginInitialize, I'm pretty certain the CDVViewController.m would fail at the canLoadRequest: check (at least in file-based apps) and then proceed to replace the WKWebViewEngine with defaultWebViewEngineClass, as commented in #7:

if ( (...) || ![self.webViewEngine canLoadRequest:[NSURLRequest requestWithURL:self.appUrl]]) {
    self.webViewEngine = [[NSClassFromString(defaultWebViewEngineClass) alloc] initWithFrame:bounds];
}

Specifically this is due to the following code in CDVWKWebViewEngine:

cordova-plugin-wkwebview-engine/src/ios/CDVWKWebViewEngine.m

Line 136 in 33a7517

return [_engineWebView respondsToSelector:wk_sel];

_engineWebView would still be nil here (because pluginInitialize wouldn't have been called), which would as far as I understand (I'm Swift-first, ObjC when necessary) be coerced to NO, making the above check fail.

As far as I can see, the only clean solution to this would be to pass settings into the WebViewEngine's init method. That is probably also the most logical and explicit way about getting the settings from config.xml into the WebView.

[shazron]

I'm creating failing unit tests for this first, I think we have passed the point where we can argue about the implementation, but we need objective verification: https://issues.apache.org/jira/browse/CB-11496

[shazron]

I commented on #7, updateWithInfo needs to be changed to not allow updating the WKWebViewConfiguration, because you can't update it at runtime -- I objectively verified this in the updated unit tests. In your commit -- basically we need updateWithInfo still, this is in the API, but it calls your updateWebViewBehaviourFromSettings now.

[shazron]

Please see #13, and let's continue discussion there.

[shazron]

Please provide feedback on #13, I will be pulling that in soon.