You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2021/01/18 13:11:50 UTC
[GitHub] [cordova-android] breautek commented on a change in pull request #1148: (android) #1002: Add Null Pointer Checks to prevent Cordova from running on a destroyed activity
breautek commented on a change in pull request #1148:
URL: https://github.com/apache/cordova-android/pull/1148#discussion_r559555080
##########
File path: framework/src/org/apache/cordova/CordovaWebViewImpl.java
##########
@@ -176,22 +176,24 @@ public void run() {
e.printStackTrace();
}
- // If timeout, then stop loading and handle error
- if (loadUrlTimeout == currentLoadUrlTimeout) {
+ // If timeout, then stop loading and handle error (if activity still exists)
+ if (loadUrlTimeout == currentLoadUrlTimeout && cordova.getActivity() != null) {
Review comment:
I wonder if we should add an else condition to log a warning/debug statement when the activity no longer exists. This may help provide better insights for developers when they hit these edge cases unexpectedly.
e.g:
```java
if (loadUrlTimeout == currentLoadUrlTimeout && cordova.getActivity() != null) {
...
} else if (cordova.getActivity() == null) {
LOG.d(TAG, "Cordova activity does not exists.");
}
##########
File path: framework/src/org/apache/cordova/CordovaWebViewImpl.java
##########
@@ -553,11 +557,13 @@ public void onPageFinishedLoading(String url) {
public void run() {
try {
Thread.sleep(2000);
- cordova.getActivity().runOnUiThread(new Runnable() {
- public void run() {
- pluginManager.postMessage("spinner", "stop");
- }
- });
+ if (cordova.getActivity() != null) {
Review comment:
If we do decide if logging debug statements is a good idea, then one should be also printed if this condition fails.
##########
File path: framework/src/org/apache/cordova/CordovaWebViewImpl.java
##########
@@ -238,7 +240,9 @@ public void showWebPage(String url, boolean openExternal, boolean clearHistory,
} else {
intent.setData(uri);
}
- cordova.getActivity().startActivity(intent);
+ if (cordova.getActivity() != null) {
Review comment:
If we do decide if logging debug statements is a good idea, then one should be also printed if this condition fails.
##########
File path: framework/src/org/apache/cordova/CordovaWebViewImpl.java
##########
@@ -176,22 +176,24 @@ public void run() {
e.printStackTrace();
}
- // If timeout, then stop loading and handle error
- if (loadUrlTimeout == currentLoadUrlTimeout) {
+ // If timeout, then stop loading and handle error (if activity still exists)
+ if (loadUrlTimeout == currentLoadUrlTimeout && cordova.getActivity() != null) {
cordova.getActivity().runOnUiThread(loadError);
}
}
};
- final boolean _recreatePlugins = recreatePlugins;
- cordova.getActivity().runOnUiThread(new Runnable() {
- public void run() {
- if (loadUrlTimeoutValue > 0) {
- cordova.getThreadPool().execute(timeoutCheck);
+ if (cordova.getActivity() != null) {
Review comment:
If we do decide if logging debug statements is a good idea, then one should be also printed if this condition fails.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org