prebid / prebid-mobile-ios

Prebid Mobile SDK for iOS applications
Apache License 2.0
46 stars 86 forks source link

Move WebView init to main thread. #989

Closed jsligh closed 1 month ago

jsligh commented 1 month ago

Closes #988

weibel commented 1 month ago

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:

I have made a solution in a branch in our own fork https://github.com/Vivino/prebid-mobile-ios/pull/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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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.