Closed Bug 722163 Opened 10 years ago Closed 9 years ago

Convert JumpListBuilder to use LazyIdleThread


(Core :: Widget: Win32, defect)

Not set





(Reporter: khuey, Assigned: malenesok007)


(Whiteboard: [mentor=khuey][lang=c++])


(1 file, 5 obsolete files)

The JumpListBuilder uses an nsThread for I/O.  (  We should use LazyIdleThread instead so that the thread will go away when not needed (

This is probably a good second bug.
Assignee: nobody → malenesok007
Attachment #599900 - Attachment is obsolete: true
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/ -f
c:\mozilla-build\python\python.exe: can't open file 'build/pymake/': [Err
no 2] No such file or directory
Attached patch Still need to test more on (obsolete) — Splinter Review
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.
Attachment #600187 - Flags: review?(khuey)
Attached patch patch_722163_v2 (obsolete) — Splinter Review
nothing change much just remove the comment line.
Attachment #600187 - Attachment is obsolete: true
Attachment #600187 - Flags: review?(khuey)
Comment on attachment 602626 [details] [diff] [review]

>@@ -82,17 +87,20 @@ JumpListBuilder::JumpListBuilder() :
>   mMaxItems(0),
>   mHasCommit(false)
> {
>   ::CoInitialize(NULL);
>   CoCreateInstance(CLSID_DestinationList, NULL, CLSCTX_INPROC_SE
>                    IID_ICustomDestinationList, getter_AddRefs(mJ
>-  NS_NewThread(getter_AddRefs(mIOThread));
>+  // 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,
>+                                             LazyIdleThread::Man

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.
Attachment #602626 - Flags: review+
Attachment #603151 - Attachment is obsolete: true
Attachment #603151 - Attachment is patch: false
Attachment #602626 - Attachment is obsolete: true
Attachment #603151 - Attachment is obsolete: false
Attachment #603151 - Attachment is patch: true
Malen, could you upload a final version of the patch that follows the instructions at That way it will be easy for someone to commit it.
Attached patch patch_722163_final_version (obsolete) — Splinter Review
This is our final version.
Attachment #603170 - Flags: review+
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).
Attachment #603151 - Attachment is obsolete: true
Attachment #603151 - Attachment is patch: false
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: - 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.
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.