From bac49fc20e7abefb5ea0faa8a6e3eae1d457fcc2 Mon Sep 17 00:00:00 2001 From: sgrebnov Date: Tue, 16 Dec 2014 11:03:58 +0300 Subject: [PATCH] CB-8139 WP8. Fix callback for plugins with native ui (capture, contactPicker, BarcodeScanner, other) Unload event could not be used to detect when CordovaView is not used anymore. For example, this event is triggered when we execute command that shows some native elements on new page and then we return back. In this case Unloaded event is called but, but control state is preserved and we should continue to use that Cordova view instance. This commit changes the following: 1. Allows command instances to be garbage collected => fixes corresponding memory leak so that NativeExecution is no more required to call DetachHandlers for each command. This fixes CB-8139. 2. Use Loaded and Unloaded events to add/remove handlers for native events; this makes it possible for GC to destroy CordovaView when it is not required anymore --- template/cordovalib/ConsoleHelper.cs | 26 ++++----- template/cordovalib/CordovaView.xaml.cs | 71 ++++++++++++------------ template/cordovalib/IBrowserDecorator.cs | 2 + template/cordovalib/NativeExecution.cs | 27 +++------ template/cordovalib/OrientationHelper.cs | 10 ++++ template/cordovalib/XHRHelper.cs | 10 ++++ 6 files changed, 75 insertions(+), 71 deletions(-) diff --git a/template/cordovalib/ConsoleHelper.cs b/template/cordovalib/ConsoleHelper.cs index 3443821c..72edfb26 100644 --- a/template/cordovalib/ConsoleHelper.cs +++ b/template/cordovalib/ConsoleHelper.cs @@ -38,13 +38,6 @@ public void InjectScript() { } - if (!hasListener) - { - PhoneApplicationService.Current.Closing += OnServiceClosing; - hasListener = true; - } - - string script = @"(function(win) { function exec(msg) { window.external.Notify('ConsoleLog/' + msg); } var cons = win.console = win.console || {}; @@ -71,15 +64,6 @@ void OnServiceClosing(object sender, ClosingEventArgs e) } } - public void DetachHandler() - { - if (hasListener) - { - PhoneApplicationService.Current.Closing -= OnServiceClosing; - hasListener = false; - } - } - public bool HandleCommand(string commandStr) { string output = commandStr.Substring("ConsoleLog/".Length); @@ -96,5 +80,15 @@ public bool HandleCommand(string commandStr) return true; } + public void AttachNativeHandlers() + { + PhoneApplicationService.Current.Closing += OnServiceClosing; + } + + public void DetachNativeHandlers() + { + PhoneApplicationService.Current.Closing -= OnServiceClosing; + } + } } diff --git a/template/cordovalib/CordovaView.xaml.cs b/template/cordovalib/CordovaView.xaml.cs index 5e790a3a..8324a613 100644 --- a/template/cordovalib/CordovaView.xaml.cs +++ b/template/cordovalib/CordovaView.xaml.cs @@ -136,22 +136,6 @@ public CordovaView() return; } - - StartupMode mode = PhoneApplicationService.Current.StartupMode; - - if (mode == StartupMode.Launch) - { - PhoneApplicationService service = PhoneApplicationService.Current; - service.Activated += new EventHandler(AppActivated); - service.Launching += new EventHandler(AppLaunching); - service.Deactivated += new EventHandler(AppDeactivated); - service.Closing += new EventHandler(AppClosing); - } - else - { - - } - // initializes native execution logic configHandler = new ConfigHandler(); configHandler.LoadAppPackageConfig(); @@ -270,6 +254,18 @@ void AppActivated(object sender, Microsoft.Phone.Shell.ActivatedEventArgs e) void CordovaBrowser_Loaded(object sender, RoutedEventArgs e) { + + PhoneApplicationService service = PhoneApplicationService.Current; + service.Activated += new EventHandler(AppActivated); + service.Launching += new EventHandler(AppLaunching); + service.Deactivated += new EventHandler(AppDeactivated); + service.Closing += new EventHandler(AppClosing); + + foreach (IBrowserDecorator iBD in browserDecorators.Values) + { + iBD.AttachNativeHandlers(); + } + this.bmHelper.ScrollDisabled = this.DisableBouncyScrolling; @@ -322,6 +318,20 @@ void CordovaBrowser_Loaded(object sender, RoutedEventArgs e) } } + private void CordovaBrowser_Unloaded(object sender, RoutedEventArgs e) + { + PhoneApplicationService service = PhoneApplicationService.Current; + service.Activated -= new EventHandler(AppActivated); + service.Launching -= new EventHandler(AppLaunching); + service.Deactivated -= new EventHandler(AppDeactivated); + service.Closing -= new EventHandler(AppClosing); + + foreach (IBrowserDecorator iBD in browserDecorators.Values) + { + iBD.DetachNativeHandlers(); + } + } + void AttachHardwareButtonHandlers() { PhoneApplicationFrame frame = Application.Current.RootVisual as PhoneApplicationFrame; @@ -524,21 +534,6 @@ public void LoadPage(string url) } } - private void CordovaBrowser_Unloaded(object sender, RoutedEventArgs e) - { - IBrowserDecorator console; - if (browserDecorators.TryGetValue("ConsoleLog", out console)) - { - ((ConsoleHelper)console).DetachHandler(); - } - - PhoneApplicationService service = PhoneApplicationService.Current; - service.Activated -= new EventHandler(AppActivated); - service.Launching -= new EventHandler(AppLaunching); - service.Deactivated -= new EventHandler(AppDeactivated); - service.Closing -= new EventHandler(AppClosing); - } - private void CordovaBrowser_NavigationFailed(object sender, System.Windows.Navigation.NavigationFailedEventArgs e) { Debug.WriteLine("CordovaBrowser_NavigationFailed :: " + e.Uri.ToString()); @@ -546,10 +541,10 @@ private void CordovaBrowser_NavigationFailed(object sender, System.Windows.Navig private void CordovaBrowser_Navigated(object sender, System.Windows.Navigation.NavigationEventArgs e) { - foreach(IBrowserDecorator iBD in browserDecorators.Values) - { - iBD.InjectScript(); - } + foreach (IBrowserDecorator iBD in browserDecorators.Values) + { + iBD.InjectScript(); + } } /// @@ -576,5 +571,11 @@ protected Color ColorFromHex(string hexString) (byte)(argb & 0xff)); return clr; } + + ~CordovaView() + { + // This trace is useful when validating that CordovaView is correctly garbage collected + // Debug.WriteLine("CordovaView is destroyed"); + } } } diff --git a/template/cordovalib/IBrowserDecorator.cs b/template/cordovalib/IBrowserDecorator.cs index bc1dbee3..84258654 100644 --- a/template/cordovalib/IBrowserDecorator.cs +++ b/template/cordovalib/IBrowserDecorator.cs @@ -26,5 +26,7 @@ interface IBrowserDecorator WebBrowser Browser { get; set; } void InjectScript(); bool HandleCommand(string cmd); + void AttachNativeHandlers(); + void DetachNativeHandlers(); } } diff --git a/template/cordovalib/NativeExecution.cs b/template/cordovalib/NativeExecution.cs index 18ca9102..fe35aea4 100644 --- a/template/cordovalib/NativeExecution.cs +++ b/template/cordovalib/NativeExecution.cs @@ -52,21 +52,6 @@ public NativeExecution(ref WebBrowser browser) this.webBrowser = browser; this.commands = new List(); - webBrowser.Unloaded += webBrowser_Unloaded; - } - - /// - /// Detaches event handlers to prevent memory leak on page navigation - /// - void webBrowser_Unloaded(object sender, RoutedEventArgs e) - { - for (int i = commands.Count - 1; i >= 0; i--) - { - if (commands[i] != null) - { - commands[i].DetachHandlers(); - } - } } /// @@ -115,6 +100,12 @@ public void ProcessCommand(CordovaCommandCall commandCallParams) return; } + // TODO: consider removing custom script functionality at all since we already marked it as absolute (see BaseCommand) + EventHandler OnCustomScriptHandler = delegate(object o, ScriptCallback script) + { + this.InvokeScriptCallback(script); + }; + EventHandler OnCommandResultHandler = delegate(object o, PluginResult res) { if (res.CallbackId == null || res.CallbackId == commandCallParams.CallbackId) @@ -123,6 +114,7 @@ public void ProcessCommand(CordovaCommandCall commandCallParams) if (!res.KeepCallback) { bc.RemoveResultHandler(commandCallParams.CallbackId); + bc.OnCustomScript -= OnCustomScriptHandler; } } }; @@ -130,11 +122,6 @@ public void ProcessCommand(CordovaCommandCall commandCallParams) //bc.OnCommandResult += OnCommandResultHandler; bc.AddResultHandler(commandCallParams.CallbackId, OnCommandResultHandler); - EventHandler OnCustomScriptHandler = delegate(object o, ScriptCallback script) - { - this.InvokeScriptCallback(script); - }; - bc.OnCustomScript += OnCustomScriptHandler; ThreadStart methodInvokation = () => diff --git a/template/cordovalib/OrientationHelper.cs b/template/cordovalib/OrientationHelper.cs index 299f9ddf..1daedf99 100644 --- a/template/cordovalib/OrientationHelper.cs +++ b/template/cordovalib/OrientationHelper.cs @@ -125,6 +125,16 @@ public bool HandleCommand(string commandStr) // No commands are currently accepted. return true; } + + public void AttachNativeHandlers() + { + // nothing to do + } + + public void DetachNativeHandlers() + { + // nothing to do + } } diff --git a/template/cordovalib/XHRHelper.cs b/template/cordovalib/XHRHelper.cs index 35913ebf..936c2699 100644 --- a/template/cordovalib/XHRHelper.cs +++ b/template/cordovalib/XHRHelper.cs @@ -342,5 +342,15 @@ public bool HandleCommand(string commandStr) return false; } + + public void AttachNativeHandlers() + { + // nothing to do + } + + public void DetachNativeHandlers() + { + // nothing to do + } } }