[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mono-winforms-list
Subject:    [Mono-winforms-list] [RFC] Multiple NativeWindows per Handle Patch
From:       "Ivan N. Zlatev" <contact () i-nz ! net>
Date:       2008-05-05 20:34:49
Message-ID: 3db1ec7f0805051334l337004fl65b0dd6c98c16a3c () mail ! gmail ! com
[Download RAW message or body]

Hey Team,

To fix bug #374660
(https://bugzilla.novell.com/show_bug.cgi?id=374660) I had to
introduce support in NativeWindow for dealing with multiple
NativeWindows per Handle, where one (the first one) is organic (the
ControlNativeWindow) and the rest are synthetic. Messages pass through
both. This is allowed in MS's implementation and is what the test code
in the bug uses to loose couple the drawing code from the behavior
code. Ugly I know, but I guess seems natural to people coming from
hardcore Win32 coding background or something.

Anyway the attached patch, which I am asking you to check out just in case:

   * All tests pass. woo! In theory no regressions.
   * Handles single handle vs multiple handles in the following way,
where window_collection is a Hashtable with key-value pair, where the
key is the handle and the value is either a single NativeWindow or an
ArrayList of handles

				object current = window_collection[handle];
				if (current != null) {
					NativeWindow currentWindow = current as NativeWindow;
					if (currentWindow != null) {
						// only one handle - do stuff
					} else { // list of windows
						ArrayList windows = (ArrayList) window_collection[handle];
						// multiple handles (first one is organic always) - do stuff
					}
				}


   * Most notably doesn't increase our current memory consumption and
should not have any performance impact.

   * Probably decreases the current memory consumption a bit. It came
to my attention that NativeWindow.FromHandle instantiates a new
NativeWindow instead of returning one from the internal table.

   * NativeWindow.window_handle: internal -> private - we don't use
this anywhere and we shouldn't provide access to this to future code
as it breaks encapuslation and allows avoiding
AssignHandle/CreateHandle, InvalidateHandle. Evil.

   * NativeWindow.windows_collection: internal -> private - now that
multiple handles are allowed and the way those are handled is an
implementation detail, code shouldn't have access to the collection
directly. I have replaced the few direct references to the field with
a method to return the organic handle.

Does anyone have any objections or should I go ahead and commit?

Thanks in advance,
Ivan

["multiple-windows-per-handle.patch" (text/x-patch)]

Index: System.Windows.Forms/LibSupport.cs
===================================================================
--- System.Windows.Forms/LibSupport.cs	(revision 102517)
+++ System.Windows.Forms/LibSupport.cs	(working copy)
@@ -40,7 +40,7 @@
 
 		static IntPtr FindWindow (IntPtr hWnd)
 		{
-			NativeWindow nw = NativeWindow.FindWindow (hWnd);
+			NativeWindow nw = NativeWindow.FromHandle (hWnd);
 			if (nw == null)
 				return IntPtr.Zero;
 
Index: System.Windows.Forms/Control.cs
===================================================================
--- System.Windows.Forms/Control.cs	(revision 102517)
+++ System.Windows.Forms/Control.cs	(working copy)
@@ -181,7 +181,7 @@
 			static internal Control ControlFromHandle(IntPtr hWnd) {
 				ControlNativeWindow	window;
 
-				window = (ControlNativeWindow)window_collection[hWnd];
+				window = (ControlNativeWindow)NativeWindow.FromHandle (hWnd);
 				if (window != null) {
 					return window.owner;
 				}
@@ -194,7 +194,7 @@
 
 				Hwnd hwnd = Hwnd.ObjectFromHandle (handle);
 				while (hwnd != null) {
-					window = (ControlNativeWindow)window_collection[hwnd.Handle];
+					window = (ControlNativeWindow)NativeWindow.FromHandle (handle);
 					if (window != null) {
 						return window.owner;
 					}
Index: System.Windows.Forms/ChangeLog
===================================================================
--- System.Windows.Forms/ChangeLog	(revision 102537)
+++ System.Windows.Forms/ChangeLog	(working copy)
@@ -1,5 +1,12 @@
 2008-05-05  Ivan N. Zlatev  <contact@i-nz.net>
 
+	* NativeWindow.cs: Add support for multiple handles per window.
+	* NativeWindows.cs, LibSupport.cs, Control.cs, XplatUIX11GTK.cs, 
+	XplatUIX11.cs, X11Display.cs: Do not access NativeWindow.windows_collection 
+	directly - use FromHandle instead.
+
+2008-05-05  Ivan N. Zlatev  <contact@i-nz.net>
+
 	* GridEntry.cs: Read-only properties with Editor with 
 	UITypeEditorEditStyle.Modal shouldn't be read-only in the PropertyGrid.
 	[Fixes bug #384184]
Index: System.Windows.Forms/XplatUIX11GTK.cs
===================================================================
--- System.Windows.Forms/XplatUIX11GTK.cs	(revision 102517)
+++ System.Windows.Forms/XplatUIX11GTK.cs	(working copy)
@@ -1408,7 +1408,7 @@
 								// Modality handling, if we are modal and the new active window is one
 								// of ours but not the modal one, switch back to the modal window
 								
-								if (NativeWindow.FindWindow (ActiveWindow) != null) {
+								if (NativeWindow.FromHandle (ActiveWindow) != null) {
 									if (ActiveWindow != (IntPtr)ModalWindows.Peek ()) {
 										Activate ((IntPtr)ModalWindows.Peek ());
 									}
Index: System.Windows.Forms/XplatUIX11.cs
===================================================================
--- System.Windows.Forms/XplatUIX11.cs	(revision 102517)
+++ System.Windows.Forms/XplatUIX11.cs	(working copy)
@@ -1783,7 +1783,7 @@
 								// Modality handling, if we are modal and the new active window is one
 								// of ours but not the modal one, switch back to the modal window
 
-								if (NativeWindow.FindWindow(ActiveWindow) != null) {
+								if (NativeWindow.FromHandle(ActiveWindow) != null) {
 									if (ActiveWindow != (IntPtr)ModalWindows.Peek()) {
 										Activate((IntPtr)ModalWindows.Peek());
 									}
Index: System.Windows.Forms/NativeWindow.cs
===================================================================
--- System.Windows.Forms/NativeWindow.cs	(revision 102517)
+++ System.Windows.Forms/NativeWindow.cs	(working copy)
@@ -17,10 +17,11 @@
 // OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 //
-// Copyright (c) 2004-2006 Novell, Inc.
+// Copyright (c) 2004-2008 Novell, Inc.
 //
 // Authors:
 //	Peter Dennis Bartok	pbartok@novell.com
+//	Ivan N. Zlatev  	<contact@i-nz.net>
 //
 
 
@@ -42,8 +43,8 @@
 		, IWin32Window
 #endif
 	{
-		internal IntPtr			window_handle;
-		static internal Hashtable	window_collection = new Hashtable();
+		IntPtr window_handle = IntPtr.Zero;
+		static Hashtable window_collection = new Hashtable();
 
 		[ThreadStatic]
 		static NativeWindow WindowCreating;
@@ -66,58 +67,108 @@
 		#region Public Static Methods
 		public static NativeWindow FromHandle(IntPtr handle)
 		{
-			NativeWindow window=new NativeWindow();
-
-			window.AssignHandle(handle);
-			return window;
+			return FindFirstInTable (handle);
 		}
 		#endregion	// Public Static Methods
 
 		#region Private and Internal Methods
-		internal static NativeWindow FindWindow(IntPtr handle)
+		internal void InvalidateHandle()
 		{
-			NativeWindow rv;
+			RemoveFromTable (this);
+			window_handle = IntPtr.Zero;
+		}
+		#endregion
+
+		#region Public Instance Methods
+		public void AssignHandle(IntPtr handle)
+		{
+			RemoveFromTable (this);
+			window_handle = handle;
+			AddToTable (this);
+			OnHandleChange();
+		}
+
+		private static void AddToTable (NativeWindow window)
+		{
+			IntPtr handle = window.Handle;
+			if (handle == IntPtr.Zero)
+				return;
+
 			lock (window_collection) {
-				rv = (NativeWindow)window_collection[handle];
+				object current = window_collection[handle];
+				if (current == null) {
+					window_collection.Add (handle, window);
+				} else {
+					NativeWindow currentWindow = current as NativeWindow;
+					if (currentWindow != null) {
+						if (currentWindow != window) {
+							ArrayList windows = new ArrayList ();
+							windows.Add (currentWindow);
+							windows.Add (window);
+							window_collection[handle] = windows;
+						}
+					} else { // list of windows
+						ArrayList windows = (ArrayList) window_collection[handle];
+						if (!windows.Contains (window))
+							windows.Add (window);
+					}
+				}
 			}
-			return rv;
 		}
 
-		internal void InvalidateHandle()
+		private static void RemoveFromTable (NativeWindow window)
 		{
+			IntPtr handle = window.Handle;
+			if (handle == IntPtr.Zero)
+				return;
+
 			lock (window_collection) {
-				window_collection.Remove(window_handle);
+				object current = window_collection[handle];
+				if (current != null) {
+					NativeWindow currentWindow = current as NativeWindow;
+					if (currentWindow != null) {
+						window_collection.Remove (handle);
+					} else { // list of windows
+						ArrayList windows = (ArrayList) window_collection[handle];
+						windows.Remove (window);
+						if (windows.Count == 0)
+							window_collection.Remove (handle);
+						else if (windows.Count == 1)
+							window_collection[handle] = windows[0];
+					}
+				}
 			}
-			window_handle = IntPtr.Zero;
 		}
-		#endregion
 
-		#region Public Instance Methods
-		public void AssignHandle(IntPtr handle)
+		private static NativeWindow FindFirstInTable (IntPtr handle)
 		{
+			if (handle == IntPtr.Zero)
+				return null;
+
+			NativeWindow window = null;
 			lock (window_collection) {
-				if (window_handle != IntPtr.Zero)
-					window_collection.Remove(window_handle);
-				window_handle=handle;
-				window_collection.Add(window_handle, this);
+				object current = window_collection[handle];
+				if (current != null) {
+					window = current as NativeWindow;
+					if (window == null) {
+						ArrayList windows = (ArrayList) current;
+						if (windows.Count > 0)
+							window = (NativeWindow) windows[0];
+					}
+				}
 			}
-			OnHandleChange();
+			return window;
 		}
 
 		public virtual void CreateHandle(CreateParams cp)
 		{
 			if (cp != null) {
 				WindowCreating = this;
-
 				window_handle=XplatUI.CreateWindow(cp);
-
 				WindowCreating = null;
 
-				if (window_handle != IntPtr.Zero) {
-					lock (window_collection) {
-						window_collection[window_handle] = this;
-					}
-				}
+				if (window_handle != IntPtr.Zero)
+					AddToTable (this);
 			}
 
 		}
@@ -136,9 +187,7 @@
 
 		public virtual void ReleaseHandle()
 		{
-			lock (window_collection) {
-				window_collection.Remove(window_handle);
-			}
+			RemoveFromTable (this);
 			window_handle=IntPtr.Zero;
 			OnHandleChange();
 		}
@@ -166,41 +215,46 @@
 
 		internal static IntPtr WndProc(IntPtr hWnd, Msg msg, IntPtr wParam, IntPtr lParam)
 		{
-			Message		m = new Message();
-			NativeWindow	window = null;
+			IntPtr result = IntPtr.Zero;
+			Message	m = new Message();
+			m.HWnd = hWnd;
+			m.Msg = (int)msg;
+			m.WParam = wParam;
+			m.LParam = lParam;
+			m.Result = IntPtr.Zero;
 					
 #if debug
 			Console.WriteLine("NativeWindow.cs ({0}, {1}, {2}, {3}): result {4}", hWnd, msg, \
wParam, lParam, m.Result);  #endif
-
-
 			//try {
-				lock (window_collection) {
-					window = (NativeWindow)window_collection[hWnd];
-				}
-				m.HWnd=hWnd;
-				m.Msg=(int)msg;
-				m.WParam=wParam;
-				m.LParam=lParam;
-				m.Result=IntPtr.Zero;
+			object current = null;
+			lock (window_collection) {
+				current = window_collection[hWnd];
+			}
 
-				if (window != null)
-					window.WndProc(ref m);
-				else if (WindowCreating != null) {
-					// we need to do this AssignHandle here instead of relying on
-					// Control.WndProc to do it, because subclasses can override
-					// WndProc, install their own WM_CREATE block, and look at
-					// this.Handle, and it needs to be set.  Otherwise, we end up
-					// recursively creating windows and emitting WM_CREATE.
-					window = WindowCreating;
-					WindowCreating = null;
-					if (window.window_handle == IntPtr.Zero)
-						window.AssignHandle (hWnd);
+			NativeWindow window = current as NativeWindow;
+			if (current == null)
+				window = EnsureCreated (window, hWnd);
 
-					window.WndProc (ref m);
+			if (window != null) {
+				window.WndProc (ref m);
+				result = m.Result;
+			} else if (current is ArrayList) {
+				ArrayList windows = (ArrayList) current;
+				lock (windows) {
+					if (windows.Count > 0) {
+						window = EnsureCreated ((NativeWindow)windows[0], hWnd);
+						window.WndProc (ref m);
+						// the first one is the control's one. all others are synthetic,
+						// so we want only the result from the control
+						result = m.Result;
+						for (int i=1; i < windows.Count; i++)
+							((NativeWindow)windows[i]).WndProc (ref m);
+					}
 				}
-				else
-					m.Result=XplatUI.DefWndProc(ref m);
+			} else {
+				result = XplatUI.DefWndProc (ref m);
+			}
 //            }
 //            catch (Exception ex) {
 //#if !ExternalExceptionHandler				
@@ -210,13 +264,28 @@
 //                throw;
 //#endif
 //            }
-
 			#if debug
 				Console.WriteLine("NativeWindow.cs: Message {0}, result {1}", msg, m.Result);
 			#endif
 
-			return m.Result;
+			return result;
 		}
+
+		private static NativeWindow EnsureCreated (NativeWindow window, IntPtr hWnd)
+		{
+			// we need to do this AssignHandle here instead of relying on
+			// Control.WndProc to do it, because subclasses can override
+			// WndProc, install their own WM_CREATE block, and look at
+			// this.Handle, and it needs to be set.  Otherwise, we end up
+			// recursively creating windows and emitting WM_CREATE.
+			if (window == null && WindowCreating != null) {
+				window = WindowCreating;
+				WindowCreating = null;
+				if (window.Handle == IntPtr.Zero)
+					window.AssignHandle (hWnd);
+			}
+			return window;
+		}
 		#endregion	// Protected Instance Methods
 	}
 }
Index: System.Windows.Forms.X11Internal/X11Display.cs
===================================================================
--- System.Windows.Forms.X11Internal/X11Display.cs	(revision 102517)
+++ System.Windows.Forms.X11Internal/X11Display.cs	(working copy)
@@ -1209,7 +1209,7 @@
 				// of ours but not the modal one, switch back to the modal window
 
 				if (ActiveWindow != null &&
-				    NativeWindow.FindWindow (ActiveWindow.Handle) != null) {
+				    NativeWindow.FromHandle (ActiveWindow.Handle) != null) {
 					if (ActiveWindow != (X11Hwnd)ModalWindows.Peek())
 						((X11Hwnd)ModalWindows.Peek()).Activate ();
 				}



_______________________________________________
Mono-winforms-list maillist  -  Mono-winforms-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-winforms-list


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic