Open Bug 683785 Opened 9 years ago Updated 2 years ago
Create timers directly instead of with do
This renames nsTimerImpl to mozilla::Timer and changes do_CreateInstance("@mozilla.org/timer;1", &err); into 'new mozilla::Timer' This has a bunch of advantages: * it's nicer to look at * it removes the need to handle do_CreateInstance() failure. (which about 1/3 of callers weren't handling) * will let compilers easily devirtualize the call to InitWithFuncCallback etc.
Attachment #557374 - Flags: feedback?(benjamin)
Comment on attachment 557374 [details] [diff] [review] Create timers directly. I'd be surprised if even modern compilers were smart enough to devirtualize InitWithFuncCallback without C++0x "final" markings. I think I'm ok with the general change here, although I'm still wondering about my XXX comment in nsTimerImpl::nsTimerImpl and whether the constructor should actually be fallible. Also this patch is missing the changes to nsTimerImpl.h/cpp, which seem like the important part ;-)
Attachment #557374 - Flags: feedback?(benjamin) → feedback+
- Fix conversion of CallCreateInstance - Add missing files - Take out some imglib stuff that snuck in
Attachment #557374 - Attachment is obsolete: true
Many of the uses of Timers don't need to share them, so I think I'm going to rework this so that you can just use a Timer as a value.
Thanks for looking at this, Anuj!
Assignee: nobody → anujagarwal464
OS: Mac OS X → All
Hardware: x86 → All
Anuj, are you still planning on working on this?
@Nathan, If somebody is working on this reassign it to them.
Thanks, Anuj. I'm going to clear the Assignee field so other people searching for C++ bugs to work on can see that this bug is available.
Assignee: anujagarwal464 → nobody
@Nathan What is it that remains in this task? I can look into it if nobody else is in line. Just give me a bit of a spec and I'll fix it.
(In reply to Linus Probert from comment #8) > @Nathan What is it that remains in this task? I can look into it if nobody > else is in line. Just give me a bit of a spec and I'll fix it. The basics are already done in attachment 558933 [details] [diff] [review]. What remains is: - Fixing up the patch for current mozilla-central (it's been a couple years)--the scripts in bug 579517 will probably be helpful here; - Replacing any new additions of |do_CreateInstance("@mozilla.org/timer;1");| with |new Timer|; - Testing the patch to ensure that it doesn't regress anything. I think the idea in comment 3 of using timers as values can wait for a followup patch.
Hi Nathan, I would like to work on this bug, can you please assign it to me?
Hello, I would like to work on this bug
(In reply to aliyua7 from comment #11) > Hello, I would like to work on this bug Hey, I am working on this bug, currently I'm busy with exams till 11th December, after that I will be working on this. And this is assigned to me also :)
Hi, can I be assigned to this bug? I can get to work on it immediately.
You need to log in before you can comment on or make changes to this bug.