The JumpListBuilder uses an nsThread for I/O. (http://mxr.mozilla.org/mozilla-central/search?string=mIOThread&find=JumpList) We should use LazyIdleThread instead so that the thread will go away when not needed (http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/LazyIdleThread.h).
This is probably a good second bug.
Created attachment 599900 [details]
Change from nsThread to LazyIdleThread in line 97
We're still testing if I works fine in windows. This is the error that we had when trying to run pymake. I also create a windows path variable to all the mozilla-build. If anyone had this problem before please let us know. Thanks.
$ python -OO build/pymake/make.py -f client.mk
c:\mozilla-build\python\python.exe: can't open file 'build/pymake/make.py': [Err
no 2] No such file or directory
Created attachment 600187 [details] [diff] [review]
Still need to test more on
We're still need to run more tests
We have been run lot of tests on a windows 32bit for the patch and It seems working fine. Does any one have a chance to test the patch please give us a feedback?. Thanks
You'll want to delete the commented out line. Let's see what Kyle thinks, otherwise.
Created attachment 602626 [details] [diff] [review]
nothing change much just remove the comment line.
Comment on attachment 602626 [details] [diff] [review]
>@@ -82,17 +87,20 @@ JumpListBuilder::JumpListBuilder() :
> CoCreateInstance(CLSID_DestinationList, NULL, CLSCTX_INPROC_SE
> IID_ICustomDestinationList, getter_AddRefs(mJ
>+ // Make a lazy thread for any IO we need (like clearing or enu
>+ // contents of indexedDB database directories).
>+ mIOThread = new LazyIdleThread(DEFAULT_THREAD_TIMEOUT_MS,
Please fix the indentation so that these are all lined up. It also looks like you lost some characters here at the end (this should be 'LazyIdleThread::ManualShutdown);').
Other than that, it looks great. Thanks for the patch.
Created attachment 603151 [details]
The correction of alignment and fix some missing on ManualShutdown
Malen, could you upload a final version of the patch that follows the instructions at https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F? That way it will be easy for someone to commit it.
Created attachment 603170 [details] [diff] [review]
This is our final version.
That didn't address the request in comment 9 - the patch doesn't include an author or commit message (something like "Bug 722163 - Convert JumpListBuilder to use LazyIdleThread. r=khuey" would be great).
Created attachment 603408 [details] [diff] [review]
Bug 722163 - Convert JumpListBuilder to use LazyIdleThread. r=khuey
To avoid further back and forth, I'm going to check in this patch and add the missing information. However, please read the link in comment 9 again - it gives specific steps for how to generate a patch that requires no modifications. Compare your patch file with this one: https://bug725430.bugzilla.mozilla.org/attachment.cgi?id=603283 - yours is missing the metadata at the top beginning that makes it easier for other people to commit it.
Thanks a lot. Now I got it.