Move WebView init to main thread.

[jsligh]

Closes #988

[weibel]

If the webview has not initialized the completion block will not be called. Is that OK?

[weibel]

Since I initiated this issue myself I have reviewed the code. Here's my feedback

There are some potential areas where improvements can be made to ensure thread safety, efficiency, and better management of the user agent string. Here are my suggestions:

• Use a synchronization mechanism like DispatchSemaphore to manage concurrent access to the user agent string.
• Persist the user agent string, to minimize fetch time on subsequent app launches. The WKWebView init and javascript evaluation on app start can take several hundred ms depending on os and device.
• Introduce a delay when initializing the WKWebView to ensure it's ready before evaluating JavaScript. (EDIT: This is probably not necessary. The registering of the webviews suggested below, seems to have solved this issue)

I have made a solution in a branch in our own fork Vivino#1 tell me if you want a PR

IMO it's a lot of work to fetch the string. Have you ever considered reverse engineering the string format and accept that it might not always reflect the string from WKWebView?

[YuriyVelichkoPI]

Hi @weibel, thanks for the review!

I like the idea to persist the ua string. However, we still should update the value because the user can update the OS, and the actual string will be changed as well.

We want to avoid blocking operations since we had already faced deadlock solving this task with semaphores. So let @OlenaPostindustria propose the fix as well, and we will choose the most reliable variant.

[weibel]

@YuriyVelichkoPI We could e.g. persist the string with the OS version as key. I tried this in the solution I linked to. It's still in review on our side.

Did you consider the following?

IMO it's a lot of work to fetch the string. Have you ever considered reverse engineering the string format and accept that it might not always reflect the string from WKWebView?

[YuriyVelichkoPI]

We could e.g. persist the string with the OS version as key. I tried this in the solution I linked to. It's still in review on our side.

Did you consider the following?

IMO it's a lot of work to fetch the string. Have you ever considered reverse engineering the string format and accept that it might not always reflect the string from WKWebView?

It should be considered on the committee. I'm not sure how sensitive this info is. In some cases it should be precies.

[OlenaPostindustria]

There is no need to keep the webView instance the whole time, so I suggest deallocating it when the user agent is fetched. But as we use singleton it can be pretty tricky and can cause problems if we accidentally deallocate the web view instance when another fetchUserAgent is in progress. My solution here would be to create the web view registry. Check out the code below for more info. Please, let me know your thoughts about this.

@objc(PBMUserAgentService) @objcMembers
public class UserAgentService: NSObject {
    
    public static let shared = UserAgentService()
    
    public private(set) var userAgent: String = ""
    
    private var webViews = [WKWebView]()
    
    override init() {
        super.init()
        
        fetchUserAgent()
    }
    
    public func fetchUserAgent(completion: ((String) -> Void)? = nil) {
        // user agent has been already generated
        guard userAgent.isEmpty else {
            completion?(userAgent)
            return
        }
        
        DispatchQueue.main.async {
            let webView = WKWebView()
            self.webViews.append(webView)
            
            webView.evaluateJavaScript("navigator.userAgent") { [weak self] result, error in
                guard let self = self else { return }
                
                if let error {
                    Log.error(error.localizedDescription)
                }
                
                if let result = result, self.userAgent.isEmpty  {
                    self.userAgent = "\(result)"
                }
                
                self.webViews.removeAll(where: { $0 == webView })
                
                completion?(self.userAgent)
            }
        }
    }
}

[YuriyVelichkoPI]

It should work well since the registry and userAgent will be managed sequentially in the main thread. userAgent should be safe for reading from any thread. Alternatively, the value passed to fetchUserAgent completion handler should be used.

@weibel also mentioned the "warming up" of the WKWebView object before runing the JS code in it. Is it possible that the result of fetching the ua will be different depending on the js execution time in the WebView lifecycle?

[jsligh]

@OlenaPostindustria thank you so much for your proposed solution! I like this method as it bypasses the proposed issues with singletons.

[OlenaPostindustria]

@YuriyVelichkoPI No, I don't think so, the result should be the same.

[weibel]

I'm unsure about the warm up time. I need to run a few tests. The problems i have had might be becasue the webview was deallocated before the block could return as i did not think about the singleton issue that was solved with registering the views

[OlenaPostindustria]

LGTM