Closed Bug 658750 Opened 10 years ago Closed 2 years ago

Make nsFrameSelection not inherit from nsISupports and de-virtualize its methods

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1345247

People

(Reporter: craig.topper, Assigned: craig.topper)

References

Details

Attachments

(2 files, 1 obsolete file)

After bug 658143, this class doesn't implement any interfaces other nsISupports and there's no need for the QI support. We can just implement AddRef/Release and remove the inheritance.

Patch coming soon.
Attachment #534202 - Flags: review?(roc)
Comment on attachment 534203 [details] [diff] [review]
Part 2: Make nsFrameSelection not inherit from nsISupports.

Hopefully I handled the cycle collection stuff correctly.
Attachment #534203 - Flags: review?(roc)
Comment on attachment 534202 [details] [diff] [review]
Part 1: Make nsFrameSelection methods non-virtual.

Review of attachment 534202 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #534202 - Flags: review?(roc) → review+
In part 2, can you use NS_INLINE_DECL_REFCOUNTING for nsFrameSelection?
If I do that, I need to move nsTypedSelection to a header file so that nsFrameSelection's destructor can destroy an nsRefPtr<nsTypedSelection>.
You should indent your cycle collection macros just like we indent QI macros.
Attachment #534203 - Attachment is obsolete: true
Attachment #534203 - Flags: review?(roc)
Comment on attachment 534253 [details] [diff] [review]
Part 2: Make nsFrameSelection not inherit from nsISupports.

Fixed indentation and made the destructor not inline so that AddRef/Release could use the inline macro.
Attachment #534253 - Flags: review?(roc)
Comment on attachment 534253 [details] [diff] [review]
Part 2: Make nsFrameSelection not inherit from nsISupports.

Review of attachment 534253 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #534253 - Flags: review?(roc) → review+
Keywords: checkin-needed
Backed out...
  http://hg.mozilla.org/projects/cedar/rev/3a058aff13ae
  http://hg.mozilla.org/projects/cedar/rev/6250f1d890d0
...on suspicion of causing leaks like this:
{
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 355851 bytes during test execution
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 130 instances of AtomImpl with size 20 bytes each (2600 bytes total)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BackstagePass with size 24 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BodyRule with size 16 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of DR_State with size 40 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of DocumentRule with size 36 bytes
}

The cycle with these patches had 3 jobs (so far) with leaks like this (2 crashtest, 1 mochitest-2 with a slightly different set of leaked objects), and the subsequent cycle had 5 jobs so far (2 crashtest, and a mochitest-1,2,3)

Also -- so far, all these leaks are on WinXP & Linux, FWIW. (That may just be coincidence, though -- it seems at least somewhat sporadic, & the other platforms' jobs may have gotten lucky.)

Error logs:
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306123416.1306123792.19004.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306121404.1306122280.12967.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306121491.1306122803.15312.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306123197.1306124918.23495.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306124839.1306125221.24769.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306124331.1306127674.1639.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306123724.1306127730.1905.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1306124750.1306126060.27547.gz
Whiteboard: fixed-in-cedar
Summary: Make nsFrameSelection not inherit from nsISupports and de-virtualize it's methods → Make nsFrameSelection not inherit from nsISupports and de-virtualize its methods
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1345247
You need to log in before you can comment on or make changes to this bug.