The current implementation fires the callback only when the network changes the first time.
Created attachment 45584 [details] The patch This patch creates a new thread only for waiting for a network change. Is there an other possibility to something like that already? Is it allowed to start a thread here? This patch also closes the handles.
style-queue ran check-webkit-style on attachment 45584 [details] without any errors.
Comment on attachment 45584 [details] The patch It's a little unfortunate to make a special thread just for this purpose. Is there another way to do it? I'm also concerned about the amount of work being done in the destructor.
(In reply to comment #3) > It's a little unfortunate to make a special thread just for this purpose. Is > there another way to do it? I tried it with the RegisterWaitForSingleObject but i didn't get it running. UnregisterWait must be called _after_ the _finished_ callback, otherwise it will return ERROR_IO_PENDING. The missing UnregisterWait seams to be the reason why it only works the first time. Even RegisterWaitForSingleObject isn't aviable unter WinCE. > I'm also concerned about the amount of work being done in the destructor. I'm too, but i didn't find a better place :-(
Using WTF threading calls instead of calling CreateThread() directly would be better, since that calls _beginthreadex where available. See http://support.microsoft.com/kb/104641
Comment on attachment 45584 [details] The patch > +NetworkStateNotifier::~NetworkStateNotifier() > +{ > + m_closeAddrChangeWaitThread = true; > + ::CloseHandle(m_overlapped.hEvent); > + ::WaitForSingleObject(m_waitHandle, INFINITE); > + ::CloseHandle(m_waitHandle); > } NetworkStateNotifier is a singleton that is never destroyed. This code will never be called. It seems like the real fix here is to call NotifyAddrChange again after we receive a notification. Why can't we just add another call to NotifyAddrChange/RegisterWaitForSingleObject inside addressChanged()?
(In reply to comment #6) > NetworkStateNotifier is a singleton that is never destroyed. This code will > never be called. I'll remove the destructor. > It seems like the real fix here is to call NotifyAddrChange again after we > receive a notification. Why can't we just add another call to > NotifyAddrChange/RegisterWaitForSingleObject inside addressChanged()? To register again you must unregister first. This can only be done outside of the callback, because UnregisterWait will return ERROR_IO_PENDING otherwise. Maybe there is a working solution with RegisterWaitForSingleObject, but i didn't tried it very hard, because RegisterWaitForSingleObject isn't available on WinCE anyway.
Created attachment 50104 [details] The patch (removed destructor)
Comment on attachment 50104 [details] The patch (removed destructor) There are so many whitespaces (and perhaps newline?) changes in here that it is hard to review/figure out what changed. Please revert the unnecessary parts of the change (even if it means using a different editor than you usually do) and keep the change focused on the fix. Also, there are changes to remove "onLine()" and "setNetworkAccessAllowed()" which is only there for QT and these seem totally unrelated to the fix. So r- due to too many unrelated changes.
Created attachment 51673 [details] The patch (removed unrelated changes)
Comment on attachment 51673 [details] The patch (removed unrelated changes) You don't seem to have incorporated previous feedback on your patch. In particular, you're still using ::CreateThread instead of WTF threads, as requested in Comment #5.
Created attachment 59208 [details] Patch (now with WTF::createThread) (In reply to comment #11) > You don't seem to have incorporated previous feedback on your patch. In particular, you're still using ::CreateThread instead of WTF threads, as requested in Comment #5. Done.
(In reply to comment #7) > (In reply to comment #6) > > It seems like the real fix here is to call NotifyAddrChange again after we > > receive a notification. Why can't we just add another call to > > NotifyAddrChange/RegisterWaitForSingleObject inside addressChanged()? > To register again you must unregister first. This can only be done outside of the callback, because UnregisterWait will return ERROR_IO_PENDING otherwise. > Maybe there is a working solution with RegisterWaitForSingleObject, but i didn't tried it very hard, because RegisterWaitForSingleObject isn't available on WinCE anyway. It looks like we don't actually need to call RegisterWaitForSingleObject. Unless you specify WT_EXECUTEONLYONCE, RegisterWaitForSingleObject will continue waiting and calling your callback every time the object is signalled (until UnregisterWait is called). So all we need to do is call NotifyAddrChanged again every time our callback is called.
I have a patch for this.
Created attachment 59256 [details] Patch
Comment on attachment 59256 [details] Patch r=me if you add a comment to addrChangeCallback!
Committed r61554: <http://trac.webkit.org/changeset/61554>
http://trac.webkit.org/changeset/61554 might have broken Tiger Intel Release The following changes are on the blame list: http://trac.webkit.org/changeset/61553 http://trac.webkit.org/changeset/61554 http://trac.webkit.org/changeset/61555 http://trac.webkit.org/changeset/61556 http://trac.webkit.org/changeset/61557