-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MOB-511 Fix JS runner in webview of older devices #109
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,12 +12,9 @@ import exchange.dydx.utilities.utils.Logging | |
import kotlinx.coroutines.CoroutineScope | ||
import kotlinx.coroutines.flow.MutableStateFlow | ||
import kotlinx.coroutines.launch | ||
import timber.log.Timber | ||
import java.util.regex.Pattern | ||
|
||
private const val TAG: String = "JavascriptRunner(V4)" | ||
private const val DO_LOG: Boolean = AppConfig.VERBOSE_LOGGING | ||
val LOGGER = if (DO_LOG) Timber.tag(TAG) else null | ||
|
||
class JavascriptRunnerV4 constructor( | ||
private val scriptDescription: String, | ||
|
@@ -28,7 +25,7 @@ class JavascriptRunnerV4 constructor( | |
|
||
override val initialized = MutableStateFlow(false) | ||
|
||
var callBackMap: MutableMap<String, ResultCallback> = mutableMapOf() | ||
private var callbackMap: MutableMap<String, ResultCallback> = mutableMapOf() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: val |
||
|
||
companion object { | ||
fun runnerFromFile( | ||
|
@@ -47,13 +44,17 @@ class JavascriptRunnerV4 constructor( | |
|
||
@JavascriptInterface | ||
fun onJsAsyncResult(key: String, result: String?) { | ||
val callback = callBackMap[key] | ||
if (callback == null) { | ||
logger.d(TAG, "No callback found for key: $key") | ||
return | ||
val callback: ResultCallback? | ||
synchronized(callbackMap) { | ||
callback = callbackMap[key] | ||
|
||
if (callback == null) { | ||
logger.e(TAG, "No callback found for key: $key") | ||
return | ||
} | ||
callbackMap.remove(key) | ||
} | ||
callBackMap.remove(key) | ||
callback.invoke(JavascriptRunnerResult(response = result)) | ||
callback?.invoke(JavascriptRunnerResult(response = result)) | ||
} | ||
|
||
override var webview: WebView? = null | ||
|
@@ -68,7 +69,7 @@ class JavascriptRunnerV4 constructor( | |
} | ||
initialized.value = false | ||
if (value != null) { | ||
// value.settings.javaScriptEnabled = true | ||
// value.settings.javaScriptEnabled = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come this isn't necessary? Also, why comment out vs just removing the code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was old code.. |
||
logger.d(TAG, "Loading base url") | ||
} | ||
|
||
|
@@ -85,6 +86,7 @@ class JavascriptRunnerV4 constructor( | |
) { | ||
logger.d(TAG, "Error in webview client: $error") | ||
} | ||
|
||
override fun onPageFinished(view: WebView, weburl: String) { | ||
initializeJavascriptEnvironment() { | ||
logger.d(TAG, "Initialized javascript environment: $it") | ||
|
@@ -96,68 +98,81 @@ class JavascriptRunnerV4 constructor( | |
|
||
override val webappInterface: WebAppInterface = WebAppInterface() | ||
|
||
override suspend fun runJs(function: String, params: List<String>, callback: ResultCallback) { | ||
override fun runJs(function: String, params: List<String>, callback: ResultCallback) { | ||
val tranformedParams: MutableList<String> = params.toMutableList() | ||
val paramsText = tranformedParams.joinToString(",") | ||
|
||
val key = "key" + System.currentTimeMillis().toString() | ||
callBackMap[key] = callback | ||
synchronized(callbackMap) { | ||
callbackMap[key] = callback | ||
} | ||
|
||
val script = """ | ||
function bridgeFunction() { | ||
$function($paramsText).then(function(result) { | ||
bridge.onJsAsyncResult("$key", result); | ||
}); | ||
try { | ||
$function($paramsText).then(function(result) { | ||
bridge.onJsAsyncResult("$key", result); | ||
}); | ||
} catch (e) { | ||
bridge.onJsAsyncResult("$key", e.toString()); | ||
} | ||
}; | ||
|
||
bridgeFunction(); | ||
""".trimIndent() | ||
|
||
runJs(script) { } | ||
launchJs(script, callback = null) // no callback, since we let onJsAsyncResult() handle it | ||
} | ||
|
||
override suspend fun runJs(script: String, callback: ResultCallback) { | ||
override fun runJs(script: String, callback: ResultCallback) { | ||
launchJs(script, callback) | ||
} | ||
|
||
fun initializeJavascriptEnvironment(callback: ResultCallback) { | ||
launchJs(scriptInitializationCode, callback) | ||
} | ||
|
||
val pattern = Pattern.compile("""^"(.*)"\$""") | ||
private fun launchJs(script: String, callback: ResultCallback) { | ||
private fun launchJs(script: String, callback: ResultCallback?) { | ||
val length = if (BuildConfig.DEBUG) 1024 else 32 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be a |
||
val localWebview = webview | ||
if (localWebview == null) { | ||
logger.e( | ||
TAG, | ||
"Unable to run function, no webview present, ${script.take(length)}", | ||
) | ||
return | ||
} | ||
logger.d(TAG, "Running script: ${script.take(length)}") | ||
|
||
scope.launch { | ||
webappInterface.callback = callback | ||
val localWebview = webview | ||
if (localWebview == null) { | ||
logger.e(TAG, "Unable to run function, no webview present, $script") | ||
return@launch | ||
} | ||
try { | ||
// for production, only log the first 32 characters to avoid logging sensitive data | ||
val length = if (BuildConfig.DEBUG) 1024 else 32 | ||
logger.d(TAG, "Running script: ${script.take(length)}") | ||
localWebview.evaluateJavascript( | ||
script, | ||
) { | ||
logger.d(TAG, "Script completed: $it") | ||
val result = it.removeSurrounding("\"") | ||
if (result != it) { | ||
logger.d(TAG, "Stripped surrounding quotes") | ||
) { evalResult -> | ||
if (callback != null) { | ||
logger.d( | ||
TAG, | ||
"Script completed: result = $evalResult from ${script.take(32)}", | ||
) | ||
val result = evalResult.removeSurrounding("\"") | ||
if (result != evalResult) { | ||
logger.d(TAG, "Stripped surrounding quotes") | ||
} | ||
callback.invoke( | ||
JavascriptRunnerResult( | ||
response = | ||
if ("null".equals(result, ignoreCase = true)) { | ||
null | ||
} else { | ||
result | ||
}, | ||
), | ||
) | ||
} | ||
callback.invoke( | ||
JavascriptRunnerResult( | ||
response = | ||
if ("null".equals(result, ignoreCase = true)) { | ||
null | ||
} else { | ||
result | ||
}, | ||
), | ||
) | ||
} | ||
} catch (e: Exception) { | ||
logger.e(TAG, "Error executing script: $script") | ||
logger.e(TAG, "Error executing script: ${script.take(32)}, error: $e") | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
package exchange.dydx.integration.javascript | ||
|
||
interface JsEngine { | ||
suspend fun runJs(function: String, params: List<String>, callback: ResultCallback) | ||
suspend fun runJs(script: String, callback: ResultCallback) | ||
fun runJs(function: String, params: List<String>, callback: ResultCallback) | ||
fun runJs(script: String, callback: ResultCallback) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for cleaning this up :)