[Mono-dev] Win Patches for Datagrid (first here) then idle

[Stifu]

Rather than including the XML in the cs file, I was thinking not relying on
XML at all.
By the way, make sure to fix the coding style.

OnMouseDown(me); -> OnMouseDown (me);
if( disposing ) -> if (disposing)
base.Dispose( disposing ); -> base.Dispose (disposing);

etc


Rob Wilkens wrote
> 
> I think so, i think we can read xml from a string using  a
> stringreader.. I just wasn't thinking it through.
> 
> Give me some time today to get that done, it's 5:40am and i haven't had
> coffee yet.
> 
> 
> 
> -Rob
> 
> On 06/19/2012 01:56 AM, Stifu wrote:
>> Can't we simplify the test to avoid having to include that XML file?
>>
>>
>> Rob Wilkens wrote
>>> I've attached the unit test, and tested it.  It consistently fails,
>>> though not always with the same exception, when run on unpatched
>>> version.  It seems to consistently pass with the patched version.  Again
>>> this was with patch 2 which i submitted a revised version earlier
>>> tonight which took out a white space change which no longer applies.
>>>
>>> This tests
>>> Bug 5487
>>> and
>>> Bug 5511
>>> and possibly even
>>> Bug 5510 (which was a random crash which i believe was related to 5511)
>>>
>>> Patch 2 is designed to fix 5511.  The 5487 testing is a bonus because
>>> its part of the test, and 5510 was one that i only reported once and
>>> never happened after i put this patch on.
>>>
>>> -Rob
>>>
>>>
>>> On 06/18/2012 05:35 PM, Stifu wrote:
>>>> If you could just write the unit test, that should be all I need.
>>>>
>>>>
>>>> Rob Wilkens wrote
>>>>> In an automatable way, yes.
>>>>>
>>>>> By hand, not that hard.
>>>>>
>>>>> I can try to describe how to test it if you need.
>>>>>
>>>>> -Rob
>>>>> On 06/18/2012 05:27 PM, Stifu wrote:
>>>>>> If it took me 3 tries to get such a simple patch in, I think I'd
>>>>>> rather
>>>>>> resign. :)
>>>>>>
>>>>>> Anyway, I'll check out the other patches another day. Is patch 2 hard
>>>>>> to
>>>>>> test? I haven't looked at it yet.
>>>>>>
>>>>>>
>>>>>> Rob Wilkens wrote
>>>>>>> Thanks for not blindly trusting me :-)
>>>>>>>
>>>>>>>
>>>>>>> On 06/18/2012 05:20 PM, Stifu wrote:
>>>>>>>> No, it's fine.
>>>>>>>>
>>>>>>>>
>>>>>>>> Rob Wilkens wrote
>>>>>>>>> Whoops a second problem with the one you posted in the commit you
>>>>>>>>> mentioned, I think if we're checking >= then we want the results
>>>>>>>>> reversed...
>>>>>>>>>
>>>>>>>>> -Rob
>>>>>>>>>
>>>>>>>>> On 06/18/2012 05:06 PM, Rob Wilkens wrote:
>>>>>>>>>> I couldn't see it before you pushed it, and may have done it
>>>>>>>>>> wrong
>>>>>>>>>> myself (I don't recall now)..  :-)
>>>>>>>>>>
>>>>>>>>>> I'm sure in the case where this version you wrote would result in
>>>>>>>>>> a
>>>>>>>>>> zero
>>>>>>>>>> result, the zero result is probably OK.
>>>>>>>>>>
>>>>>>>>>> The version below is probably good if you're ok with it.
>>>>>>>>>>
>>>>>>>>>> -Rob
>>>>>>>>>>
>>>>>>>>>> On 06/18/2012 04:58 PM, Stifu wrote:
>>>>>>>>>>> Hah, that thought did cross my mind.
>>>>>>>>>>> Did you just wait for me to push it before saying that? :p
>>>>>>>>>>>
>>>>>>>>>>> We could go for:
>>>>>>>>>>>
>>>>>>>>>>> 			int next_pixel_offset = pixel_offset;
>>>>>>>>>>>
>>>>>>>>>>> 			if (CurrentColumn < CurrentTableStyle.GridColumnStyles.Count)	
>>>>>>>>>>> 			{
>>>>>>>>>>> 				next_pixel_offset +=
>>>>>>>>>>> CurrentTableStyle.GridColumnStyles[CurrentColumn].Width;
>>>>>>>>>>> 			}
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Rob Wilkens wrote
>>>>>>>>>>>> Slight issue which probably will never be a problem:
>>>>>>>>>>>>
>>>>>>>>>>>> I'd replace the "0" with "pixel_offset" since the normal
>>>>>>>>>>>> condition
>>>>>>>>>>>> for
>>>>>>>>>>>> that is pixel_offset PLUS the current column (which may be
>>>>>>>>>>>> invalid)
>>>>>>>>>>>> width.. 
>>>>>>>>>>>>
>>>>>>>>>>>> I don't think it'll be a problem though and can probably stand
>>>>>>>>>>>> as-is.
>>>>>>>>>>>>
>>>>>>>>>>>> -Rob
>>>>>>>>>>>>
>>>>>>>>>>>> On 06/18/2012 04:39 PM, Stifu wrote:
>>>>>>>>>>>>> Alright, the first patch is in
>>>>>>>>>>>>> (https://github.com/mono/mono/commit/42ebb31fc143a171a6a5930bc647627c557842ee).
>>>>>>>>>>>>> I took the liberty to change the coding style.
>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Rob Wilkens wrote
>>>>>>>>>>>>>> This is for Stifu:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Please follow this sequence when applying or testing the
>>>>>>>>>>>>>> patches
>>>>>>>>>>>>>> listed
>>>>>>>>>>>>>> below.  Doing in other order may break things.  If you want
>>>>>>>>>>>>>> me
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>> create
>>>>>>>>>>>>>> a unit test for something you don't see a unit test for, let
>>>>>>>>>>>>>> me
>>>>>>>>>>>>>> know,
>>>>>>>>>>>>>> but in some cases, clicks are required with a mouse and i'm
>>>>>>>>>>>>>> not
>>>>>>>>>>>>>> necessarily sure how to create a patch to do that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Ok, I've attached the patches i had queued as separate
>>>>>>>>>>>>>> individual
>>>>>>>>>>>>>> patches, i hope i did this right..  These are from ranges of
>>>>>>>>>>>>>> git
>>>>>>>>>>>>>> diffs..  Please let me know if there are issues, my feelings
>>>>>>>>>>>>>> won't
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> hurt, i'd rather do this right than do it fast.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The order to apply them in (then i'll get into what it fixes
>>>>>>>>>>>>>> after):
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'd suggest the DataGrid patches first because they are in
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> middle
>>>>>>>>>>>>>> of
>>>>>>>>>>>>>> everything and get in the way -- except don't apply the
>>>>>>>>>>>>>> IdleAndDataGrid.Whitespace.Jun10.patch until you've applied
>>>>>>>>>>>>>> ALL
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> patches prior to Jun 10 (including idle patches), those
>>>>>>>>>>>>>> patches
>>>>>>>>>>>>>> in
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> Whitespace patch don't fix anything other the prettying up
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> code,
>>>>>>>>>>>>>> but
>>>>>>>>>>>>>> they depend on both set of patches in sequence..
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> So the sequence i'm suggesting they must be applied in if
>>>>>>>>>>>>>> they
>>>>>>>>>>>>>> are
>>>>>>>>>>>>>> applied at all are:
>>>>>>>>>>>>>> (1) DataGrid1.Jun3.patch first
>>>>>>>>>>>>>> (2) DataGrid2.Jun4.patch second
>>>>>>>>>>>>>> (3) DataGrid3.Novell322563.jun4.patch third
>>>>>>>>>>>>>> (4) DataGrid4.Novell322154.jun6.patch
>>>>>>>>>>>>>> -- but don't do the other one i said not to do at this point
>>>>>>>>>>>>>> --
>>>>>>>>>>>>>> now to the idle fixes, these next ones (5-9) are meant to all
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> applied
>>>>>>>>>>>>>> as part of essentially one patch for it to work, but is
>>>>>>>>>>>>>> broken
>>>>>>>>>>>>>> up
>>>>>>>>>>>>>> so
>>>>>>>>>>>>>> you
>>>>>>>>>>>>>> can see progression.
>>>>>>>>>>>>>> (5) Idle1-3.Jun2 (sorry for forgetting patch extension), This
>>>>>>>>>>>>>> contains 3
>>>>>>>>>>>>>> commits in one but they were all related, and makes life
>>>>>>>>>>>>>> easier
>>>>>>>>>>>>>> by
>>>>>>>>>>>>>> being
>>>>>>>>>>>>>> summarized into one like this.
>>>>>>>>>>>>>> (6) Now you should do IdleAndDataGrid.Whitespace.Jun10.patch
>>>>>>>>>>>>>> (7) Next, do Idle-Win32IdleEnable.jun11.patch
>>>>>>>>>>>>>> (8) Idle-RaceConditionFix-Jun12.patch is next
>>>>>>>>>>>>>> (9) Idle-TestFixForIdle.jun12.patch is last
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> There, I have 9 attachments, and above is the sequence to
>>>>>>>>>>>>>> apply
>>>>>>>>>>>>>> them
>>>>>>>>>>>>>> in.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The below numbering system matches the above patch order
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #1: from the commit message:
>>>>>>>>>>>>>> The sample code in
>>>>>>>>>>>>>> https://bugzilla.novell.com/show_bug.cgi?id=MONO79788 was
>>>>>>>>>>>>>> crashing
>>>>>>>>>>>>>> on
>>>>>>>>>>>>>> me
>>>>>>>>>>>>>> if I clicked on a row header (where the + was). I
>>>>>>>>>>>>>> investigated
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> found
>>>>>>>>>>>>>> that it was because, when the table had no data to display
>>>>>>>>>>>>>> yet,
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> if
>>>>>>>>>>>>>> you clicked on a row header (that's the area to the left of
>>>>>>>>>>>>>> what
>>>>>>>>>>>>>> would
>>>>>>>>>>>>>> be the data), as part of assigning the current cell, it would
>>>>>>>>>>>>>> call
>>>>>>>>>>>>>> ensure cell visibility function, which would call
>>>>>>>>>>>>>> ScrollToColumnInPixels
>>>>>>>>>>>>>> which would try to get the next pixel offset by looking at
>>>>>>>>>>>>>> CurrentTableStyle.GridColumnStyles[CurrentColumn].Width, but
>>>>>>>>>>>>>> when
>>>>>>>>>>>>>> no
>>>>>>>>>>>>>> data was being displayed there were no columns. So while
>>>>>>>>>>>>>> current
>>>>>>>>>>>>>> column
>>>>>>>>>>>>>> had a value of zero, there were no items in the
>>>>>>>>>>>>>> GridColumnStyles
>>>>>>>>>>>>>> Array/List, even at zero index. The fix for this was before
>>>>>>>>>>>>>> indexing
>>>>>>>>>>>>>> into GridColumnStyles to Check The Length to make sure we're
>>>>>>>>>>>>>> not
>>>>>>>>>>>>>> going
>>>>>>>>>>>>>> beyond its bounds. It is probably perfectly acceptable if
>>>>>>>>>>>>>> we're
>>>>>>>>>>>>>> beyond
>>>>>>>>>>>>>> the bounds to just leave this value at zero for the offset.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #2 From the commit message:
>>>>>>>>>>>>>> Xamaring bug 5511: This fixes this and some side issues..
>>>>>>>>>>>>>> First, fixed the crash by creating two additional stacks for
>>>>>>>>>>>>>> when
>>>>>>>>>>>>>> navigating to and from other sub tables... Both were 'style'
>>>>>>>>>>>>>> stacks
>>>>>>>>>>>>>> which tracked per column styles. Those needed to be updated
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> reset
>>>>>>>>>>>>>> on
>>>>>>>>>>>>>> a per table basis. Second, When navigating forward or back,
>>>>>>>>>>>>>> EndEdit
>>>>>>>>>>>>>> needed to be called so that we don't leave an editing cell
>>>>>>>>>>>>>> open
>>>>>>>>>>>>>> when
>>>>>>>>>>>>>> we
>>>>>>>>>>>>>> navigate, otherwise there was the possibility and reality
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> edited cell would still be visible and editing on the next
>>>>>>>>>>>>>> table
>>>>>>>>>>>>>> either
>>>>>>>>>>>>>> forward or back. To recreate this on the sample code,
>>>>>>>>>>>>>> presuming
>>>>>>>>>>>>>> you
>>>>>>>>>>>>>> can
>>>>>>>>>>>>>> get past the initial crash which was fixed here, this could
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> illustrated without the endedits that were added as follows :
>>>>>>>>>>>>>> Run:
>>>>>>>>>>>>>> 1.
>>>>>>>>>>>>>> click + 2. click pb 3. click + 4. click pb 3. click + 5.
>>>>>>>>>>>>>> click
>>>>>>>>>>>>>> pd
>>>>>>>>>>>>>> 6.
>>>>>>>>>>>>>> click back 6. click pc 7. See 0 highlighted in column header
>>>>>>>>>>>>>> from
>>>>>>>>>>>>>> previous table Finally, I modified some previous submissions
>>>>>>>>>>>>>> on
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>> related problem so that they had more "Love for Spaces"
>>>>>>>>>>>>>> (whitespace)
>>>>>>>>>>>>>> without changing the actual code other than that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The above may have fixed, i think it was 5510 too.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #3: From the commit
>>>>>>>>>>>>>> This solves PART of Novell Bugzilla Issue #322563
>>>>>>>>>>>>>> https://bugzilla.novell.com/show_bug.cgi?id=322563 What this
>>>>>>>>>>>>>> accomplishes is that it hides the non browsable columns
>>>>>>>>>>>>>> (columns
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> were not part of the original dataset, in the test case) from
>>>>>>>>>>>>>> view
>>>>>>>>>>>>>> in
>>>>>>>>>>>>>> the DataGrid. Unfortunately, i can't see an obvious way to
>>>>>>>>>>>>>> hide
>>>>>>>>>>>>>> the'parent rows' display of those non browsable columns
>>>>>>>>>>>>>> value.
>>>>>>>>>>>>>> That
>>>>>>>>>>>>>> is
>>>>>>>>>>>>>> if You viewed a subtable off hidden field pb_Id=0 it would
>>>>>>>>>>>>>> show
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> value (pb_Id=0) on the top of the datagrid where it shows the
>>>>>>>>>>>>>> previous
>>>>>>>>>>>>>> rows. The remaining issue seems to be a non major issue since
>>>>>>>>>>>>>> it
>>>>>>>>>>>>>> is
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>> non functional issue.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #4: From the commit
>>>>>>>>>>>>>> Novell #323154
>>>>>>>>>>>>>> Decided to include this in same branch (same pull request) as
>>>>>>>>>>>>>> earlier
>>>>>>>>>>>>>> changes due to it affecting the same DataGrid.cs -- i didn't
>>>>>>>>>>>>>> want
>>>>>>>>>>>>>> any
>>>>>>>>>>>>>> conflicts. Here's a copy of what i wrote up earlier about
>>>>>>>>>>>>>> this:
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> problem report is here:
>>>>>>>>>>>>>> https://bugzilla.novell.com/show_bug.cgi?id=323154 I found by
>>>>>>>>>>>>>> deduction
>>>>>>>>>>>>>> that the repaint wasn't cancelling the active edit box after
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> row
>>>>>>>>>>>>>> was
>>>>>>>>>>>>>> deleted .. So while the table updated, the edit box with the
>>>>>>>>>>>>>> old
>>>>>>>>>>>>>> value
>>>>>>>>>>>>>> didn't go away... The repaint was initiated from an
>>>>>>>>>>>>>> Invalidate
>>>>>>>>>>>>>> call
>>>>>>>>>>>>>> which stack trace looked something like this:
>>>>>>>>>>>>>> System.Windows.Forms.DataGrid.CalcAreasAndInvalidate() at
>>>>>>>>>>>>>> System.Windows.Forms.DataGrid.RecreateDataGridRows(Boolean
>>>>>>>>>>>>>> recalc)
>>>>>>>>>>>>>> at
>>>>>>>>>>>>>> System.Windows.Forms.DataGrid.OnListManagerItemChanged(System.Object
>>>>>>>>>>>>>> sender, System.Windows.Forms.ItemChangedEventArgs e) at
>>>>>>>>>>>>>> System.Windows.Forms.CurrencyManager.OnItemChanged(System.Windows.Forms.ItemChangedEventArgs
>>>>>>>>>>>>>> e) at System.Windows.Forms.CurrencyManager.UpdateIsBinding()
>>>>>>>>>>>>>> at
>>>>>>>>>>>>>> System.Windows.Forms.CurrencyManager.ListChangedHandler(System.Object
>>>>>>>>>>>>>> sender, System.ComponentModel.ListChangedEventArgs e) at
>>>>>>>>>>>>>> System.Data.DataView.OnListChanged(System.ComponentModel.ListChangedEventArgs
>>>>>>>>>>>>>> e) at System.Data.DataView.OnRowDeleted(System.Object sender,
>>>>>>>>>>>>>> System.Data.DataRowChangeEventArgs args) at
>>>>>>>>>>>>>> System.Data.DataTable.OnRowDeleted(System.Data.DataRowChangeEventArgs
>>>>>>>>>>>>>> e)
>>>>>>>>>>>>>> at System.Data.DataTable.DeletedDataRow(System.Data.DataRow
>>>>>>>>>>>>>> dr,
>>>>>>>>>>>>>> DataRowAction action) at System.Data.DataRow.Delete() at
>>>>>>>>>>>>>> System.Data.DataView.Delete(Int32 index) at
>>>>>>>>>>>>>> System.Data.DataRowView.Delete() at
>>>>>>>>>>>>>> grid.ProcessCmdKey(Message
>>>>>>>>>>>>>> ByRef
>>>>>>>>>>>>>> msg, Keys keyData) (etc) ProcessCmdKey is from user code in
>>>>>>>>>>>>>> sample
>>>>>>>>>>>>>> in
>>>>>>>>>>>>>> bug report... After the delete (as seen above), the first
>>>>>>>>>>>>>> thing
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> DataGrid gets back is an OnListManagerItemChanged... Before
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> would
>>>>>>>>>>>>>> call RecreateDataGridRows(), if it was going to do that, i
>>>>>>>>>>>>>> inserted
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>> check to see if we're editing, and if so, i cancel the edit
>>>>>>>>>>>>>> (because
>>>>>>>>>>>>>> we're reloading dataset), here is a summary of what my patch
>>>>>>>>>>>>>> will
>>>>>>>>>>>>>> look
>>>>>>>>>>>>>> like in ONListManagerItemChanged in DataGrid.cs in
>>>>>>>>>>>>>> System.Windows.Forms
>>>>>>>>>>>>>> directory: if (rows == null || RowsCount != rows.Length -
>>>>>>>>>>>>>> (ShowEditRow ?
>>>>>>>>>>>>>> 1 : 0)) + { + if (is_editing) + CancelEditing ();
>>>>>>>>>>>>>> RecreateDataGridRows
>>>>>>>>>>>>>> (true); + } This solved the problem reported. It is now
>>>>>>>>>>>>>> identical
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>> windows .net behavior from what i can see, as far as this
>>>>>>>>>>>>>> problem
>>>>>>>>>>>>>> report
>>>>>>>>>>>>>> goes anyway. MS .NET Crashes as well as mono in the sample
>>>>>>>>>>>>>> code
>>>>>>>>>>>>>> if
>>>>>>>>>>>>>> you
>>>>>>>>>>>>>> press a key twice to delete two rows when there was only one
>>>>>>>>>>>>>> row
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>> delete (index out of range). This is not necessarily a good
>>>>>>>>>>>>>> thing,
>>>>>>>>>>>>>> but
>>>>>>>>>>>>>> it's user code from the bug report which causes that, not
>>>>>>>>>>>>>> anything
>>>>>>>>>>>>>> inherently different in mono.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #5-#9 are all about fixing bug From Novell #321541 which if i
>>>>>>>>>>>>>> have
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> right number is adding the ability to have the idle event
>>>>>>>>>>>>>> handler
>>>>>>>>>>>>>> only
>>>>>>>>>>>>>> send idle events to the thread the idle handler was assigned
>>>>>>>>>>>>>> in. 
>>>>>>>>>>>>>> So
>>>>>>>>>>>>>> if
>>>>>>>>>>>>>> you have 2 threads, and they each assigned an idle event
>>>>>>>>>>>>>> handler,
>>>>>>>>>>>>>> they
>>>>>>>>>>>>>> would each get their own idle event handler called when that
>>>>>>>>>>>>>> thread
>>>>>>>>>>>>>> went
>>>>>>>>>>>>>> idle.  Or if only one thread had an idle event handler
>>>>>>>>>>>>>> assigned
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> same idle handler wouldn't be called for EVERY thread, it
>>>>>>>>>>>>>> would
>>>>>>>>>>>>>> only
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> called on the thread it was assigned on.  This is so it more
>>>>>>>>>>>>>> closely
>>>>>>>>>>>>>> matches the windows .net behavior.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'll include each of the individual commit messages though
>>>>>>>>>>>>>> they
>>>>>>>>>>>>>> were
>>>>>>>>>>>>>> all
>>>>>>>>>>>>>> towards the same goal
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #5 had three commits:
>>>>>>>>>>>>>> This addresses a 6-year old Novell bugzilla issue: 321541...
>>>>>>>>>>>>>> Created a hashtable of per-thread event handlers for idle..
>>>>>>>>>>>>>> Assigned
>>>>>>>>>>>>>> in
>>>>>>>>>>>>>> to that hashtable when the regular Idle eventhandler was
>>>>>>>>>>>>>> assigned
>>>>>>>>>>>>>> by
>>>>>>>>>>>>>> mapping it by ManagedThreadId. The hashtable had to use a
>>>>>>>>>>>>>> separate
>>>>>>>>>>>>>> object (different class) rather than an EventHandler
>>>>>>>>>>>>>> directly,
>>>>>>>>>>>>>> because
>>>>>>>>>>>>>> an eventhandler apparently cannot be assigned to another
>>>>>>>>>>>>>> object,
>>>>>>>>>>>>>> it
>>>>>>>>>>>>>> can
>>>>>>>>>>>>>> only be a part of a class. It also couldn't be checked for
>>>>>>>>>>>>>> null
>>>>>>>>>>>>>> or
>>>>>>>>>>>>>> called from outside the class so i handled that as
>>>>>>>>>>>>>> appropriate
>>>>>>>>>>>>>> (by
>>>>>>>>>>>>>> secondary callers). This has been checked against the code in
>>>>>>>>>>>>>> 321541
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> the code appears to work fine now. I have also confirmed no
>>>>>>>>>>>>>> new
>>>>>>>>>>>>>> unit
>>>>>>>>>>>>>> test failures have been introduced by this change. There were
>>>>>>>>>>>>>> 3
>>>>>>>>>>>>>> failures
>>>>>>>>>>>>>> before, and are three failures now. Also, This includes a
>>>>>>>>>>>>>> unit
>>>>>>>>>>>>>> test,
>>>>>>>>>>>>>> which will fail without this patch. Here's a shortcut to the
>>>>>>>>>>>>>> Novell
>>>>>>>>>>>>>> bug:
>>>>>>>>>>>>>> https://bugzilla.novell.com/show_bug.cgi?id=321541
>>>>>>>>>>>>>> Per suggestion, Changed a Hashtable to a generic dictionary.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This change is to properly use the GenericDictionary to use
>>>>>>>>>>>>>> the EventHandler type directly rather than the temporary
>>>>>>>>>>>>>> class
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> was
>>>>>>>>>>>>>> being used in my first attempt at this.
>>>>>>>>>>>>>> (so some of the changes from the first commit you don't see
>>>>>>>>>>>>>> here
>>>>>>>>>>>>>> because
>>>>>>>>>>>>>> they were thrown away by the second and third in a rewrite)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #6 is a bunch of whitespace fixes for Datagrid and idle which
>>>>>>>>>>>>>> has
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> applied at this point in sequence.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #7 Enables the idle message to work on Win32.  When i tested
>>>>>>>>>>>>>> on
>>>>>>>>>>>>>> Win32
>>>>>>>>>>>>>> i
>>>>>>>>>>>>>> realized the Idle event handler was never called, so i fixed
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> so
>>>>>>>>>>>>>> when it went idle it would call it.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Here's the commit from #7:
>>>>>>>>>>>>>> This patch will enable the idle event to be called when the
>>>>>>>>>>>>>> applicati...
>>>>>>>>>>>>>> ...on is idle on Win32. This was necessary to make an earlier
>>>>>>>>>>>>>> unit
>>>>>>>>>>>>>> test
>>>>>>>>>>>>>> from the same set of patches work on win32. Plus, it's a good
>>>>>>>>>>>>>> idea.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #8 This is important, essentially this had a lock set to stop
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>> race
>>>>>>>>>>>>>> condition that was happening with the test. There was an 'if
>>>>>>>>>>>>>> something
>>>>>>>>>>>>>> == null assign something to something new... And two threads
>>>>>>>>>>>>>> were
>>>>>>>>>>>>>> hitting this code at the same time and this was causing one
>>>>>>>>>>>>>> thread
>>>>>>>>>>>>>> to
>>>>>>>>>>>>>> assign it, and before it would start working with it, thread
>>>>>>>>>>>>>> 2
>>>>>>>>>>>>>> would
>>>>>>>>>>>>>> reassign it, and this resulted in a stack dump (exception) in
>>>>>>>>>>>>>> add_Idle,
>>>>>>>>>>>>>> this fix seemed to stop that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Here's the commit message
>>>>>>>>>>>>>> This code fixes a possible race condition which during my
>>>>>>>>>>>>>> testing
>>>>>>>>>>>>>> seemed
>>>>>>>>>>>>>> to be hit about once every twenty runs or so. Sometimes if
>>>>>>>>>>>>>> two
>>>>>>>>>>>>>> threads
>>>>>>>>>>>>>> were assigning the idle handler at the same time, and it's
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> first
>>>>>>>>>>>>>> assignment, they will both try to assign a new dictionary.
>>>>>>>>>>>>>> This
>>>>>>>>>>>>>> resulted
>>>>>>>>>>>>>> in funny behavior, such as immediately after an add, the item
>>>>>>>>>>>>>> wouldn't
>>>>>>>>>>>>>> be found. After applying this fix, a lock around that
>>>>>>>>>>>>>> particular
>>>>>>>>>>>>>> check/assignment, I reran the tests about 50-75 times and
>>>>>>>>>>>>>> never
>>>>>>>>>>>>>> ran
>>>>>>>>>>>>>> into
>>>>>>>>>>>>>> this race condition again.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #9 the last one is just some test changes after what i was
>>>>>>>>>>>>>> told
>>>>>>>>>>>>>> about
>>>>>>>>>>>>>> mono requiring that all threads create the forms in the same
>>>>>>>>>>>>>> thread.
>>>>>>>>>>>>>> This only seemed to be a problem on windows, from what i
>>>>>>>>>>>>>> recall,
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> only occasionally.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The commit message reads:
>>>>>>>>>>>>>> Due to a WinForms requriement (which only seems to
>>>>>>>>>>>>>> occasionally
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>> problem on Windows), where all Controls (including Forms)
>>>>>>>>>>>>>> must
>>>>>>>>>>>>>> be
>>>>>>>>>>>>>> created on One Thread, and then to do work on them from other
>>>>>>>>>>>>>> threads
>>>>>>>>>>>>>> only by use of invoke ( According to :
>>>>>>>>>>>>>> http://www.mono-project.com/FAQ:_Winforms ), I modified my
>>>>>>>>>>>>>> unit
>>>>>>>>>>>>>> test
>>>>>>>>>>>>>> accordingly to be in compliance.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>>> Mono-devel-list mailing list
>>>>>>>>>>>>>> Mono-devel-list at .ximian
>>>>>>>>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>>>>>>>>>
>>>>>>>>>>>>> --
>>>>>>>>>>>>> View this message in context:
>>>>>>>>>>>>> http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650048.html
>>>>>>>>>>>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>>> Mono-devel-list mailing list
>>>>>>>>>>>>> Mono-devel-list at .ximian
>>>>>>>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> Mono-devel-list mailing list
>>>>>>>>>>>> Mono-devel-list at .ximian
>>>>>>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> View this message in context:
>>>>>>>>>>> http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650052.html
>>>>>>>>>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> Mono-devel-list mailing list
>>>>>>>>>>> Mono-devel-list at .ximian
>>>>>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>>>> _______________________________________________
>>>>>>>>> Mono-devel-list mailing list
>>>>>>>>> Mono-devel-list at .ximian
>>>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>>>>
>>>>>>>> --
>>>>>>>> View this message in context:
>>>>>>>> http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650057.html
>>>>>>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>>>>>>> _______________________________________________
>>>>>>>> Mono-devel-list mailing list
>>>>>>>> Mono-devel-list at .ximian
>>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>> _______________________________________________
>>>>>>> Mono-devel-list mailing list
>>>>>>> Mono-devel-list at .ximian
>>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>>>
>>>>>> --
>>>>>> View this message in context:
>>>>>> http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650059.html
>>>>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>>>>> _______________________________________________
>>>>>> Mono-devel-list mailing list
>>>>>> Mono-devel-list at .ximian
>>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>> _______________________________________________
>>>>> Mono-devel-list mailing list
>>>>> Mono-devel-list at .ximian
>>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>>>
>>>> --
>>>> View this message in context:
>>>> http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650061.html
>>>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>>>> _______________________________________________
>>>> Mono-devel-list mailing list
>>>> Mono-devel-list at .ximian
>>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>
>>> _______________________________________________
>>> Mono-devel-list mailing list
>>> Mono-devel-list at .ximian
>>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>>>
>>
>> --
>> View this message in context:
>> http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650068.html
>> Sent from the Mono - Dev mailing list archive at Nabble.com.
>> _______________________________________________
>> Mono-devel-list mailing list
>> Mono-devel-list at .ximian
>> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> 
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at .ximian
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> 


--
View this message in context: http://mono.1490590.n4.nabble.com/Win-Patches-for-Datagrid-first-here-then-idle-tp4650027p4650072.html
Sent from the Mono - Dev mailing list archive at Nabble.com.