Skip to content
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

InAppBrowser will not destroy WebView after being closed (by invoking ref.close()) causing memory leaks #290

Open
keithdmoore opened this issue Aug 31, 2018 · 33 comments · May be fixed by #882 or #982

Comments

@keithdmoore
Copy link

On both Android and iOS, ref.close() does not close the window. I can see the webview is still running. Does not seem to be specific to device or OS. However, I have been testing on iPhone X, with iOS 11.4.1 as well as Samsung S5 running Android 6.0.1.

@janpio
Copy link
Member

janpio commented Aug 31, 2018

What happens instead? Does it just stay open?

@keithdmoore
Copy link
Author

Yes. It just stays open.

@heidji
Copy link

heidji commented Oct 10, 2018

+1

before installing inAppBrowser ref.close() would work on Android. Now it doesn't.

@janpio
Copy link
Member

janpio commented Oct 30, 2018

Some context please: What is this ref.close() you refer to? You used this before using inappbrowser @heidji?

@heidji
Copy link

heidji commented Oct 30, 2018

import {InAppBrowser} from "@ionic-native/in-app-browser";
....
let ref = window.open('https://www.example.com/', '_system');
ref.close(); //does nothing

yes, i had to install the plugin because in iOS window.open doesn't work without having InAppBrowser installed, so before the plugin ref.close() in this scenario would work. now it doesn't

@heidji
Copy link

heidji commented Nov 16, 2018

@janpio I am sorry the @ionic-native plugin used in this example could be confusing, i also tested the matter by calling the cordova plugin directly using cordova.InAppBrowser.open and trying to close it also without success, se yeah it's the definitely the plugin's fault.

@janpio
Copy link
Member

janpio commented Nov 16, 2018

Ok, so https://github.com/apache/cordova-plugin-inappbrowser#inappbrowserclose does not actually work but leave the InAppBrowser window that was opened, open?

It would be awesome if one of you both could create a reproduction app with cordova create, implement the minimal code needed to show the problem and put it on GitHub.

@heidji
Copy link

heidji commented Nov 16, 2018

sure, go at it, doesn't work:

/*
 * Licensed to the Apache Software Foundation (ASF) under one
 * or more contributor license agreements.  See the NOTICE file
 * distributed with this work for additional information
 * regarding copyright ownership.  The ASF licenses this file
 * to you under the Apache License, Version 2.0 (the
 * "License"); you may not use this file except in compliance
 * with the License.  You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing,
 * software distributed under the License is distributed on an
 * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
 * KIND, either express or implied.  See the License for the
 * specific language governing permissions and limitations
 * under the License.
 */
var app = {
    // Application Constructor
    initialize: function() {
        document.addEventListener('deviceready', this.onDeviceReady.bind(this), false);
    },

    // deviceready Event Handler
    //
    // Bind any cordova events here. Common events are:
    // 'pause', 'resume', etc.
    onDeviceReady: function() {
        this.receivedEvent('deviceready');
    },

    // Update DOM on a Received Event
    receivedEvent: function(id) {
        var parentElement = document.getElementById(id);
        var listeningElement = parentElement.querySelector('.listening');
        var receivedElement = parentElement.querySelector('.received');

        listeningElement.setAttribute('style', 'display:none;');
        receivedElement.setAttribute('style', 'display:block;');

        console.log('Received Event: ' + id);

        setTimeout(() => test(), 3000);
    }


};
test = function () {
    var ref = cordova.InAppBrowser.open('https://www.google.com', '_system');
    setTimeout(() => ref.close(), 3000);
}
app.initialize();

@heidji
Copy link

heidji commented Nov 16, 2018

i also created a rep for you, please forgive me if i added too many files in there, i really have no idea about cordova in particular and it doesn't have a built in gitignore file: replink

@janpio
Copy link
Member

janpio commented Nov 16, 2018

Thanks, the repo is perfect.

While checking it out, I had a look at the documentation. Something popped up at me:

The object returned from a call to cordova.InAppBrowser.open when the target is set to '_blank'.
https://github.com/apache/cordova-plugin-inappbrowser#inappbrowser

Several of the methods mention that they only work with _blank, and the .close example also uses _blank.

This might be the first thing to investigate.

@heidji
Copy link

heidji commented Nov 16, 2018

i also suspected something similar, i think i can live with _blank :D

@heidji
Copy link

heidji commented Nov 16, 2018

@janpio actually the behavior of _blank isn't useful to me, it opens inside the app itself and doesn't close either..
i mean it uses the app as a browser

@janpio
Copy link
Member

janpio commented Nov 16, 2018

I can reproduce the behaviour you describe in the unchanged app.

After adding some debugging and changing to _blank the IAB opens and closes automatically. I pushed it to my fork: https://github.com/janpio/inappbrowsertest/tree/blank Can you confirm the IAB windows closing after a few seconds?

@heidji
Copy link

heidji commented Nov 16, 2018

actually i was mistaken about _blank not closing, but it's because also in my app _blank doesn't work: the reason being that the external http location having a callback with myapp:// scheme to return to the app, but in _blank this call never gets through. yes your rep works, but _blank doesn't work for me, it has to be an external browser with _system in order to call the custom URL scheme.

@janpio
Copy link
Member

janpio commented Nov 16, 2018

@keithdmoore Did you original issue also refer to IAB using _system?

The next step to debug this would be to see what happens on the native side of things - close seems to be handled by this code for Android:

public void closeDialog() {
this.cordova.getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
final WebView childView = inAppWebView;
// The JS protects against multiple calls, so this should happen only when
// closeDialog() is called by other native code.
if (childView == null) {
return;
}
childView.setWebViewClient(new WebViewClient() {
// NB: wait for about:blank before dismissing
public void onPageFinished(WebView view, String url) {
if (dialog != null) {
dialog.dismiss();
dialog = null;
}
}
});
// NB: From SDK 19: "If you call methods on WebView from any thread
// other than your app's UI thread, it can cause unexpected results."
// http://developer.android.com/guide/webapps/migrating.html#Threads
childView.loadUrl("about:blank");
try {
JSONObject obj = new JSONObject();
obj.put("type", EXIT_EVENT);
sendUpdate(obj, false);
} catch (JSONException ex) {
LOG.d(LOG_TAG, "Should never happen");
}
}
});
}

Unfortunately I am not set up for this.

@heidji I understand this doesn't match your usecase - maybe open a new issue describing it again and asking for solutions (I might have an idea).

@heidji
Copy link

heidji commented Nov 16, 2018

@janpio okay thanks, will open an issue for my use case

@keithdmoore
Copy link
Author

@janpio I am using ‘_blank’ and the close method leaves the inappbrowser running. It’s hidden but it’s still running. Having these running will cause memory issues

@janpio janpio changed the title InAppBrowser will not close when invoking ref.close() InAppBrowser will not destroy WebView when invoking ref.close() Nov 16, 2018
@janpio
Copy link
Member

janpio commented Nov 16, 2018

Ok, so you actually have a different "problem" than @heidji had - good that we moved his to new issues. I updated the title of this issue - is this correct?

How did you discover that the webview is still running exactly?

Update: I could reproduce what you describe in Android and the Chrome dev tools:
image
Each execution of test() from @heidji's test repository opens a new IAB windows that first loads google.com and that switches to about:blank when the IAB windows is closed via .close. That's not good.
Does this match what you are seeing and reported here?

@keithdmoore
Copy link
Author

keithdmoore commented Nov 17, 2018

@janpio that’s pretty close to what I was seeing. It’s been awhile since I looked into this issue

@janpio janpio changed the title InAppBrowser will not destroy WebView when invoking ref.close() InAppBrowser will not destroy WebView after being closed (by invoking ref.close()) Nov 17, 2018
@janpio janpio changed the title InAppBrowser will not destroy WebView after being closed (by invoking ref.close()) InAppBrowser will not destroy WebView after being closed (by invoking ref.close()) causing memory leaks Nov 17, 2018
@janpio janpio added bug and removed support labels Nov 17, 2018
@sagrawal31
Copy link

Any workaround for this?

@sagrawal31
Copy link

I'm doing some manual cleanup & closing the webview as well wizpanda/cordova-plugin-overappbrowser@b8ad90d#diff-c61103e023e9dff1bbbb02ae97772d78R374

Things working for me now.

@daffinm
Copy link

daffinm commented Mar 21, 2019

I am seeing this too on iOS 12 and Android 8. Currently running in to it on iOS 12 during app testing.

Here is the code I am using to open and close InAppBrowser windows:

let iab = cordova.InAppBrowser.open(url, '_blank', 'location=no,footer=yes'); iab.addEventListener("exit", function () { iab.close(); })

I am monitoring the windows from the Safari Develop menu and initially (after starting the app for a test run) the InAppBrowser windows are opening and closing/disappearing from the list as you would expect. And then for some reason they stop disappearing when closed and start accumulating like this:

InAppBrowser ghost windows bug

You can see one line where it says about:blank. But there are other lines with no text one of which I have highlighted in this screen shot.

I know when this is happening because the InAppBrowser window shows no content. It is just blank. I have to restart the app in order to fix things.

Does anyone have any idea why this is happening or what can be done about it?

@sagrawal31 I looked at your code which seems to be for Android only. I am running in to this problem on both iOS and Android.

Initially InAppBrowser windows seem to disappear when closed, and I have added

@daffinm
Copy link

daffinm commented Mar 22, 2019

Interestingly, on Android 8 (Webview 72.0.3626.121) I see a similar build up of _blank windows until I get to between 5 and 7. Then suddenly most of the dead windows seem go get garbage collected. I'll check iOS 12...

I cannot now reproduce the original problem on iOS 12, and there is no build up of dead _blank windows, at least this is not visible via the Safari Develop menu. The only thing I changed was to move the iab reference up and make it an object field rather than a local variable. I will continue testing and see if it happens again on iOS.

After more testing on iOS 12 there seems to be a causal relationship between html5 video and IAB. My app plays an internal video on the welcome screen. This is configured to play inline but sometimes it goes fullscreen when I press play. After I close the fullscreen video and open IAB I sometimes get the a blank white window. Sometimes closing the fullscreen video crashes the app, and when this happens IAB is always blank after I restart the app. I then have to reinstall the app (not tried rebooting) to reset things. I am now experimenting with using Video.js to see if this stops my app crashing when I play video and also cures this IAB issue.

@daffinm
Copy link

daffinm commented Mar 23, 2019

@keithdmoore et al. The problem is reappearing again on iOS. I thought it was related to running html5 video in the app (not in the IAB window - in the main webview) because html5 video was causing my app to crash when closing fullscreen mode. I can reliably reproduce this IAB problem on iOS 12 by opening a video in fullscreen and closing it three or four times until the app crashes. After starting the app again all IAB windows (which I am using for displaying internal documentation) are white and I start seeing a build up of ghost windows in the Safari Develop sub menu for the device. Interestingly, the forward and back controls in the bottom bar (RHS) of the IAB window go from grey to blue but do not respond to clicks. This behaviour continues until either the app is reinstalled or the device is rebooted.

Given that html5 video is unusable in my Cordova app I have removed all video. After doing this I continued testing the app and again found that the IAB windows started displaying no content, just a white empty screen with nothing in the document and no error messages. Not sure what caused this. I was connected to the app with Safari in order to debug, and I was messing with css issues editing CSS live in the Safari debugger. Beyond this I was just navigating between pages in the app (using Bootstrap Carousel for now). I am now going to stop using IAB, uninstall it and code an alternative for displaying my internal docs as I need to release asap and cannot afford to send it out misbehaving like this on iOS. I don't know what else to do as this issue is beyond my current abilities to debug.

@robinson29a
Copy link

robinson29a commented Oct 2, 2019

I made this change in the closeDialog function and it worked for me, I also recommend waiting about three seconds after calling the close function before calling the function to open again.

childView.setWebViewClient(new WebViewClient() {
     // NB: wait for about:blank before dismissing
     public void onPageFinished(WebView view, String url) {
         if (dialog != null) {
             dialog.dismiss();
             dialog = null;
         }
	 inAppWebView.destroy();
      }
});

@HarshRohila
Copy link

HarshRohila commented Oct 29, 2019

                    // NB: wait for about:blank before dismissing
                    public void onPageFinished(WebView view, String url) {
                        if (dialog != null) {
                            dialog.dismiss();
                            dialog = null;
                        }
                        if (url.equals(new String("about:blank"))) {
							inAppWebView.onPause();
							inAppWebView.removeAllViews();
							inAppWebView.destroyDrawingCache();
							inAppWebView.destroy();
							inAppWebView = null;
						}
                    }
                });

Above worked for me
References: @robinson29a comment
https://stackoverflow.com/questions/17418503/destroy-webview-in-android/17458577 (following steps from here to close webview)

@bvx89
Copy link

bvx89 commented Jan 23, 2020

Is there any update to this? I've tested the code that @HarshRohila posted, and it worked without a problem in my cordova app.

@asanka-indrajith
Copy link

any fix for iOS?

wif70068 added a commit to wif70068/cordova-plugin-inappbrowser that referenced this issue Aug 9, 2020
@mashoodpv
Copy link

mashoodpv commented Sep 21, 2020

Same issue here:

Screen Shot 2020-09-21 at 8 06 07 AM

@Midi86
Copy link

Midi86 commented Mar 22, 2021

By analyzing the native code for IOS (of version 3.2) I found that the close event is only triggered while the browser is in the process of being hidden. If the browser is already hidden - for example through the options property - the close call method will not be called. Thus a quick fix for IOS was for me to call show() just before closing the browser:

ref.show(); ref.close();

@marshall86
Copy link

By analyzing the native code for IOS (of version 3.2) I found that the close event is only triggered while the browser is in the process of being hidden. If the browser is already hidden - for example through the options property - the close call method will not be called. Thus a quick fix for IOS was for me to call show() just before closing the browser:

ref.show(); ref.close();

This solution seems to work even though it's not that nice for the user to see even for a second the browser opening and closing..

Any better option? it's quite annoying

PDLMobileApps pushed a commit to PDLMobileApps/cordova-plugin-inappbrowser that referenced this issue Jun 16, 2021
PDLMobileApps pushed a commit to PDLMobileApps/cordova-plugin-inappbrowser that referenced this issue Jun 16, 2021
jeansparipassu added a commit to paripassubr/cordova-plugin-inappbrowser that referenced this issue Nov 8, 2021
leonardo-fernandes added a commit to leonardo-fernandes/cordova-plugin-inappbrowser that referenced this issue Dec 13, 2021
Whenever closing an InAppBrowser instance, a webview was left in memory with about:blank page.
This change fixes the issue by destroying and freeing the inAppWebView object.
@v1934
Copy link

v1934 commented Jun 23, 2022

It's 2022 and same thing still happens to me on Android. Jeez, cordova is such a dead thing. Literally every plugin is outdated by at least 2 or 3 years and not maintained, with 15 forks floating around the internet but nobody even cares to accept the pull requests.

alexgerardojacinto added a commit to OutSystems/cordova-plugin-inappbrowser that referenced this issue Aug 30, 2023
…ty (#42)

* Fix memory leak - initially reported in apache#290

Whenever closing an InAppBrowser instance, a webview was left in memory with about:blank page.
This change fixes the issue by destroying and freeing the inAppWebView object.

* Fix beforeLoad not being called in some requests - initially reported in apache#686

The waitForBeforeload flag was preventing beforeLoad from being called on every GET request.

* Do not lose callbackContext when opening a SYSTEM url

This fixes a condition where it was not possible to open a SYSTEM url while an InAppBrowser instance is displayed. The callbackContext of the InAppBrowser instance was lost when the SYSTEM url was opened. This fixes the issue by not setting the callbackContext on SYSTEM urls.

* Fix crash when pausing/resuming application after closing InAppBrowser

* Fix _loadAfterBeforeload callback not working due to this.rootName not being defined.

* Reset beforeload variable if a new instance of InAppBrowser is opened without beforeload setting

* chore: update changelog

References: https://outsystemsrd.atlassian.net/browse/RMET-2802

* refactor: remove empty lines

* refactor: use ternary operator and remove unnecessary ones

Why: We can refactor the if-else block that assigns a value to beforeload to use a ternary operator. On the other hand, we can remove the unnecessary ternary operators used to assign a boolean value. They are redundant.

References: https://outsystemsrd.atlassian.net/browse/RMET-2802

---------

Co-authored-by: Leonardo Monteiro Fernandes <leonardo.monteiro.fernandes@gmail.com>
Co-authored-by: Nelson Lopes Silva <5671236+nflsilva@users.noreply.github.com>
ralphcode added a commit to ralphcode/cordova-plugin-inappbrowser that referenced this issue Aug 31, 2023
Fix for webView window not being destroyed correctly causing memory leak (apache/cordova-plugin-inappbrowser#290)
alexgerardojacinto added a commit to OutSystems/cordova-plugin-inappbrowser that referenced this issue Aug 31, 2023
* RMET-2119 InAppBrowser Plugin - Check for element before obtaining it (#40)

* fix: check if there's a next token before trying to obtain it

References: https://outsystemsrd.atlassian.net/browse/RMET-2119

* chore: update changelog

* RMET-2802 InAppBrowser Plugin - Multiple minor fixes from the community (#42)

* Fix memory leak - initially reported in apache#290

Whenever closing an InAppBrowser instance, a webview was left in memory with about:blank page.
This change fixes the issue by destroying and freeing the inAppWebView object.

* Fix beforeLoad not being called in some requests - initially reported in apache#686

The waitForBeforeload flag was preventing beforeLoad from being called on every GET request.

* Do not lose callbackContext when opening a SYSTEM url

This fixes a condition where it was not possible to open a SYSTEM url while an InAppBrowser instance is displayed. The callbackContext of the InAppBrowser instance was lost when the SYSTEM url was opened. This fixes the issue by not setting the callbackContext on SYSTEM urls.

* Fix crash when pausing/resuming application after closing InAppBrowser

* Fix _loadAfterBeforeload callback not working due to this.rootName not being defined.

* Reset beforeload variable if a new instance of InAppBrowser is opened without beforeload setting

* chore: update changelog

References: https://outsystemsrd.atlassian.net/browse/RMET-2802

* refactor: remove empty lines

* refactor: use ternary operator and remove unnecessary ones

Why: We can refactor the if-else block that assigns a value to beforeload to use a ternary operator. On the other hand, we can remove the unnecessary ternary operators used to assign a boolean value. They are redundant.

References: https://outsystemsrd.atlassian.net/browse/RMET-2802

---------

Co-authored-by: Leonardo Monteiro Fernandes <leonardo.monteiro.fernandes@gmail.com>
Co-authored-by: Nelson Lopes Silva <5671236+nflsilva@users.noreply.github.com>

---------

Co-authored-by: Leonardo Monteiro Fernandes <leonardo.monteiro.fernandes@gmail.com>
Co-authored-by: Nelson Lopes Silva <5671236+nflsilva@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment