add JS bridge to webviews and run inject JS performance tracking #809
Conversation
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/webview/WebViewMessageHandler.kt
Outdated
Show resolved
Hide resolved
platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/webview/WebViewCapture.kt
Show resolved
Hide resolved
|
|
||
| try { | ||
| val script = WebViewBridgeScript.SCRIPT | ||
| WebViewCompat.addDocumentStartJavaScript( |
There was a problem hiding this comment.
wouldn't we be calling this in addition to webview.addJavascriptInterface() for webviews that support both?
There was a problem hiding this comment.
Yeah, that's what's happening at the moment. The instrument function hooks up the bridge via addJavascriptInterface, then calls this injectScript function which handles injecting all the OOTB javascript handlers.
I do see that AI left a spot for a fallback injection method, and then didn't implement it, so I'll do that.
There was a problem hiding this comment.
ah I see, the 2 calls are needed. Yeah curious how it would behave if for some reason WebViewFeature.isFeatureSupported(WebViewFeature.DOCUMENT_START_SCRIPT) returns false
There was a problem hiding this comment.
Added the fallback approach: 005249a
Probably worth considering whether we even care to support the case where DOCUMENT_START_SCRIPT is unsupported? I'll add a discussion point in the PRD doc
jacksonhardaker
force-pushed
the
jh/js-bridge-webview
branch
from
5a19170 to
d0e196a
Compare
| val original = WebViewCompat.getWebViewClient(webview) | ||
| WebViewCapture(original, effectiveLogger, needsFallback) | ||
| } else { | ||
| WebViewCapture(WebViewClient(), effectiveLogger, needsFallback) |
There was a problem hiding this comment.
not sure how this all works but I worry that there might be cases where WebViewFeature.isFeatureSupported(WebViewFeature.GET_WEB_VIEW_CLIENT) returns false but the user still provided their own custom WebViewClient and we might be just blindly overriding it
| let navigationDelegate = WebViewNavigationDelegate( | ||
| original: webView.navigationDelegate, | ||
| logger: effectiveLogger, | ||
| messageHandler: capture.messageHandler | ||
| ) | ||
| capture.navigationDelegate = navigationDelegate | ||
| webView.navigationDelegate = navigationDelegate |
There was a problem hiding this comment.
its not clear to me that this delegate object would not get garbage collected, particularly under memory pressure, since webView.navigationDelegate is a weak ref -- might have to hold on to these? Same for capture.
alternately there could be one delegate object that is always around and it keeps a reference to the various webviews and associated data like page load times.
|
|
||
| /// Tracks instrumented WebViews to prevent double-instrumentation | ||
| private static var instrumentedWebViews = NSHashTable<WKWebView>.weakObjects() | ||
| private static let lock = NSLock() |
There was a problem hiding this comment.
I'd probably give this a more specific name, since its a lock for the instrumented web views collection in particular and who knows, might lock something else.
Android Benchmark Results
Allocations
Timing
|
Example session opening amazon.com: https://timeline.bitdrift.dev/session/c321a55f-c7cd-49cf-a99b-e4e34fd4f03e?utm_source=sdk&utilization=0#timeline
Example session opening wikipedia.org: https://timeline.bitdrift.dev/session/7049a0c4-20b9-44c8-8325-2087967f3d04?utm_source=sdk&utilization=0#timeline