cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From purplecabb...@apache.org
Subject [1/3] cordova-wp8 git commit: CB-8139 WP8. Fix callback for plugins with native ui (capture, contactPicker, BarcodeScanner, other)
Date Tue, 16 Dec 2014 08:05:14 GMT
Repository: cordova-wp8
Updated Branches:
  refs/heads/master 6bb3fc38a -> 9979b938c


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


Project: http://git-wip-us.apache.org/repos/asf/cordova-wp8/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-wp8/commit/1215509c
Tree: http://git-wip-us.apache.org/repos/asf/cordova-wp8/tree/1215509c
Diff: http://git-wip-us.apache.org/repos/asf/cordova-wp8/diff/1215509c

Branch: refs/heads/master
Commit: 1215509c58f6568e06aa6707e389f877513824f8
Parents: 6bb3fc3
Author: sgrebnov <v-segreb@microsoft.com>
Authored: Mon Dec 15 17:52:27 2014 +0300
Committer: sgrebnov <v-segreb@microsoft.com>
Committed: Mon Dec 15 17:52:27 2014 +0300

----------------------------------------------------------------------
 template/cordovalib/ConsoleHelper.cs     | 26 ++++------
 template/cordovalib/CordovaView.xaml.cs  | 70 ++++++++++++++-------------
 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(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-wp8/blob/1215509c/template/cordovalib/ConsoleHelper.cs
----------------------------------------------------------------------
diff --git a/template/cordovalib/ConsoleHelper.cs b/template/cordovalib/ConsoleHelper.cs
index 3443821..72edfb2 100644
--- a/template/cordovalib/ConsoleHelper.cs
+++ b/template/cordovalib/ConsoleHelper.cs
@@ -38,13 +38,6 @@ namespace WPCordovaClassLib.CordovaLib
             {
             }
 
-            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 @@ namespace WPCordovaClassLib.CordovaLib
             }
         }
 
-        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 @@ namespace WPCordovaClassLib.CordovaLib
             return true;
         }
 
+        public void AttachNativeHandlers()
+        {
+            PhoneApplicationService.Current.Closing += OnServiceClosing;
+        }
+
+        public void DetachNativeHandlers()
+        {
+            PhoneApplicationService.Current.Closing -= OnServiceClosing;
+        }
+
     }
 }

http://git-wip-us.apache.org/repos/asf/cordova-wp8/blob/1215509c/template/cordovalib/CordovaView.xaml.cs
----------------------------------------------------------------------
diff --git a/template/cordovalib/CordovaView.xaml.cs b/template/cordovalib/CordovaView.xaml.cs
index 5e790a3..4c509e0 100644
--- a/template/cordovalib/CordovaView.xaml.cs
+++ b/template/cordovalib/CordovaView.xaml.cs
@@ -136,21 +136,7 @@ namespace WPCordovaClassLib
                 return;
             }
 
-
-            StartupMode mode = PhoneApplicationService.Current.StartupMode;
-
-            if (mode == StartupMode.Launch)
-            {
-                PhoneApplicationService service = PhoneApplicationService.Current;
-                service.Activated += new EventHandler<Microsoft.Phone.Shell.ActivatedEventArgs>(AppActivated);
-                service.Launching += new EventHandler<LaunchingEventArgs>(AppLaunching);
-                service.Deactivated += new EventHandler<DeactivatedEventArgs>(AppDeactivated);
-                service.Closing += new EventHandler<ClosingEventArgs>(AppClosing);
-            }
-            else
-            {
-
-            }
+            Debug.WriteLine("Created new CordovaView instance");
 
             // initializes native execution logic
             configHandler = new ConfigHandler();
@@ -270,6 +256,18 @@ namespace WPCordovaClassLib
 
         void CordovaBrowser_Loaded(object sender, RoutedEventArgs e)
         {
+          
+            PhoneApplicationService service = PhoneApplicationService.Current;
+            service.Activated += new EventHandler<Microsoft.Phone.Shell.ActivatedEventArgs>(AppActivated);
+            service.Launching += new EventHandler<LaunchingEventArgs>(AppLaunching);
+            service.Deactivated += new EventHandler<DeactivatedEventArgs>(AppDeactivated);
+            service.Closing += new EventHandler<ClosingEventArgs>(AppClosing);
+
+            foreach (IBrowserDecorator iBD in browserDecorators.Values)
+            {
+                iBD.AttachNativeHandlers();
+            }
+
 
             this.bmHelper.ScrollDisabled = this.DisableBouncyScrolling;
 
@@ -322,6 +320,20 @@ namespace WPCordovaClassLib
             }
         }
 
+        private void CordovaBrowser_Unloaded(object sender, RoutedEventArgs e)
+        {
+            PhoneApplicationService service = PhoneApplicationService.Current;
+            service.Activated -= new EventHandler<Microsoft.Phone.Shell.ActivatedEventArgs>(AppActivated);
+            service.Launching -= new EventHandler<LaunchingEventArgs>(AppLaunching);
+            service.Deactivated -= new EventHandler<DeactivatedEventArgs>(AppDeactivated);
+            service.Closing -= new EventHandler<ClosingEventArgs>(AppClosing);
+
+            foreach (IBrowserDecorator iBD in browserDecorators.Values)
+            {
+                iBD.DetachNativeHandlers();
+            }
+        }
+
         void AttachHardwareButtonHandlers()
         {
             PhoneApplicationFrame frame = Application.Current.RootVisual as PhoneApplicationFrame;
@@ -524,21 +536,6 @@ namespace WPCordovaClassLib
             }
         }
 
-        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<Microsoft.Phone.Shell.ActivatedEventArgs>(AppActivated);
-            service.Launching -= new EventHandler<LaunchingEventArgs>(AppLaunching);
-            service.Deactivated -= new EventHandler<DeactivatedEventArgs>(AppDeactivated);
-            service.Closing -= new EventHandler<ClosingEventArgs>(AppClosing);
-        }
-
         private void CordovaBrowser_NavigationFailed(object sender, System.Windows.Navigation.NavigationFailedEventArgs
e)
         {
             Debug.WriteLine("CordovaBrowser_NavigationFailed :: " + e.Uri.ToString());
@@ -546,10 +543,10 @@ namespace WPCordovaClassLib
 
         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();
+            }
         }
 
         /// <summary>
@@ -576,5 +573,10 @@ namespace WPCordovaClassLib
                               (byte)(argb & 0xff));
             return clr;
         }
+
+        ~CordovaView()
+        {
+            Debug.WriteLine("CordovaView is destroyed");
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/cordova-wp8/blob/1215509c/template/cordovalib/IBrowserDecorator.cs
----------------------------------------------------------------------
diff --git a/template/cordovalib/IBrowserDecorator.cs b/template/cordovalib/IBrowserDecorator.cs
index bc1dbee..8425865 100644
--- a/template/cordovalib/IBrowserDecorator.cs
+++ b/template/cordovalib/IBrowserDecorator.cs
@@ -26,5 +26,7 @@ namespace WPCordovaClassLib.CordovaLib
         WebBrowser Browser { get; set; }
         void InjectScript();
         bool HandleCommand(string cmd);
+        void AttachNativeHandlers();
+        void DetachNativeHandlers();
     }
 }

http://git-wip-us.apache.org/repos/asf/cordova-wp8/blob/1215509c/template/cordovalib/NativeExecution.cs
----------------------------------------------------------------------
diff --git a/template/cordovalib/NativeExecution.cs b/template/cordovalib/NativeExecution.cs
index 18ca910..fe35aea 100644
--- a/template/cordovalib/NativeExecution.cs
+++ b/template/cordovalib/NativeExecution.cs
@@ -52,21 +52,6 @@ namespace WPCordovaClassLib.Cordova
 
             this.webBrowser = browser;
             this.commands = new List<BaseCommand>();
-            webBrowser.Unloaded += webBrowser_Unloaded;
-        }
-
-        /// <summary>
-        /// Detaches event handlers to prevent memory leak on page navigation
-        /// </summary>
-        void webBrowser_Unloaded(object sender, RoutedEventArgs e)
-        {
-            for (int i = commands.Count - 1; i >= 0; i--)
-            {
-                if (commands[i] != null)
-                {
-                    commands[i].DetachHandlers();
-                }
-            }
         }
 
         /// <summary>
@@ -115,6 +100,12 @@ namespace WPCordovaClassLib.Cordova
                     return;
                 }
 
+                // TODO: consider removing custom script functionality at all since we already
marked it as absolute (see BaseCommand)
+                EventHandler<ScriptCallback> OnCustomScriptHandler = delegate(object
o, ScriptCallback script)
+                {
+                    this.InvokeScriptCallback(script);
+                };
+
                 EventHandler<PluginResult> OnCommandResultHandler = delegate(object
o, PluginResult res)
                 {
                     if (res.CallbackId == null || res.CallbackId == commandCallParams.CallbackId)
@@ -123,6 +114,7 @@ namespace WPCordovaClassLib.Cordova
                         if (!res.KeepCallback)
                         {
                             bc.RemoveResultHandler(commandCallParams.CallbackId);
+                            bc.OnCustomScript -= OnCustomScriptHandler;
                         }
                     }
                 };
@@ -130,11 +122,6 @@ namespace WPCordovaClassLib.Cordova
                 //bc.OnCommandResult += OnCommandResultHandler;
                 bc.AddResultHandler(commandCallParams.CallbackId, OnCommandResultHandler);
 
-                EventHandler<ScriptCallback> OnCustomScriptHandler = delegate(object
o, ScriptCallback script)
-                {
-                    this.InvokeScriptCallback(script);
-                };
-
                 bc.OnCustomScript += OnCustomScriptHandler;
 
                 ThreadStart methodInvokation = () =>

http://git-wip-us.apache.org/repos/asf/cordova-wp8/blob/1215509c/template/cordovalib/OrientationHelper.cs
----------------------------------------------------------------------
diff --git a/template/cordovalib/OrientationHelper.cs b/template/cordovalib/OrientationHelper.cs
index 299f9dd..0ec32fe 100644
--- a/template/cordovalib/OrientationHelper.cs
+++ b/template/cordovalib/OrientationHelper.cs
@@ -125,6 +125,16 @@ namespace WPCordovaClassLib.Cordova
             // No commands are currently accepted.
             return true;
         }
+
+        public void AttachNativeHandlers()
+        {
+            // nothing todo
+        }
+
+        public void DetachNativeHandlers()
+        {
+            // nothing to do
+        }
     }
 
 

http://git-wip-us.apache.org/repos/asf/cordova-wp8/blob/1215509c/template/cordovalib/XHRHelper.cs
----------------------------------------------------------------------
diff --git a/template/cordovalib/XHRHelper.cs b/template/cordovalib/XHRHelper.cs
index 35913eb..1d72367 100644
--- a/template/cordovalib/XHRHelper.cs
+++ b/template/cordovalib/XHRHelper.cs
@@ -342,5 +342,15 @@ namespace WPCordovaClassLib.CordovaLib
 
             return false;
         }
+
+        public void AttachNativeHandlers()
+        {
+            // nothing todo
+        }
+
+        public void DetachNativeHandlers()
+        {
+            // nothing to do
+        }
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org


Mime
View raw message