apache / cordova-plugin-inappbrowser

Apache Cordova InAppBrowser Plugin
https://cordova.apache.org/
Apache License 2.0
1.12k stars 2.15k forks source link

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

Open keithdmoore opened 6 years ago

keithdmoore commented 6 years ago

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 commented 6 years ago

What happens instead? Does it just stay open?

keithdmoore commented 6 years ago

Yes. It just stays open.

heidji commented 6 years ago

+1

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

janpio commented 5 years ago

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

heidji commented 5 years ago
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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

heidji commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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: https://github.com/apache/cordova-plugin-inappbrowser/blob/3eadde65ab0bcd17cc2cd0231b1d8a3f68bbd5b8/src/android/InAppBrowser.java#L509-L543 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 commented 5 years ago

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

keithdmoore commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

sagrawal31 commented 5 years ago

Any workaround for this?

sagrawal31 commented 5 years ago

We are opening a few HTML5 games and after launching 2-3 games, app is getting crashed because of memory issues.

sagrawal31 commented 5 years ago

I'm doing some manual cleanup & closing the webview as well https://github.com/wizpanda/cordova-plugin-overappbrowser/commit/b8ad90dfb5488fac7037c7c86b146159d5796176#diff-c61103e023e9dff1bbbb02ae97772d78R374

Things working for me now.

daffinm commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 4 years ago
                    // 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 commented 4 years ago

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 commented 4 years ago

any fix for iOS?

mashoodpv commented 4 years ago

Same issue here:

Screen Shot 2020-09-21 at 8 06 07 AM
Midi86 commented 3 years ago

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 commented 3 years ago

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

v1934 commented 2 years ago

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.

ilyakamens commented 3 weeks ago

This did the trick for me for iOS: https://github.com/Something-Useful/cordova-plugin-inappbrowser/commit/61720c6caba695ddf2204d9c80ec2153d1218415