Last Comment Bug 658143 - Stop creating nsFrameSelection with CreateInstance
: Stop creating nsFrameSelection with CreateInstance
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Craig Topper
:
Mentors:
Depends on:
Blocks: 658750
  Show dependency treegraph
 
Reported: 2011-05-18 19:19 PDT by Craig Topper
Modified: 2011-05-20 23:08 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: Changes nsCOMPtr<nsFrameSelection> to nsRefPtr. (24.95 KB, patch)
2011-05-18 20:06 PDT, Craig Topper
no flags Details | Diff | Review
Part 1: Changes nsCOMPtr<nsFrameSelection> to nsRefPtr. (24.93 KB, patch)
2011-05-18 20:11 PDT, Craig Topper
roc: review+
Details | Diff | Review
Part 2: Don't use createInstance for nsFrameSelection. (3.88 KB, patch)
2011-05-18 22:28 PDT, Craig Topper
roc: review+
Details | Diff | Review
Part 3: Remove nsFrameSelection QI and createInstance support. (8.37 KB, patch)
2011-05-18 22:37 PDT, Craig Topper
roc: review+
Details | Diff | Review
Part 3: Remove nsFrameSelection QI and createInstance support. (8.37 KB, patch)
2011-05-19 22:41 PDT, Craig Topper
craig.topper: review+
Details | Diff | Review

Description Craig Topper 2011-05-18 19:19:14 PDT
nsFrameSelection is created in two places with createInstance. Both places are inside the layout module and can just create nsFrameSelection directly.

This removes the need to be able to create nsFrameSelection with createInstance. Then nsFrameSelection no longer needs an IID since there are no other QIs.
Comment 1 Craig Topper 2011-05-18 20:06:22 PDT
Created attachment 533515 [details] [diff] [review]
Part 1: Changes nsCOMPtr<nsFrameSelection> to nsRefPtr.
Comment 2 Craig Topper 2011-05-18 20:11:25 PDT
Created attachment 533517 [details] [diff] [review]
Part 1: Changes nsCOMPtr<nsFrameSelection> to nsRefPtr.
Comment 3 Craig Topper 2011-05-18 22:28:47 PDT
Created attachment 533537 [details] [diff] [review]
Part 2: Don't use createInstance for nsFrameSelection.
Comment 4 Craig Topper 2011-05-18 22:37:37 PDT
Created attachment 533539 [details] [diff] [review]
Part 3: Remove nsFrameSelection QI and createInstance support.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-18 22:43:15 PDT
Comment on attachment 533517 [details] [diff] [review]
Part 1: Changes nsCOMPtr<nsFrameSelection> to nsRefPtr.

Review of attachment 533517 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-18 22:43:57 PDT
Comment on attachment 533537 [details] [diff] [review]
Part 2: Don't use createInstance for nsFrameSelection.

Review of attachment 533537 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-05-18 22:44:51 PDT
Comment on attachment 533539 [details] [diff] [review]
Part 3: Remove nsFrameSelection QI and createInstance support.

Review of attachment 533539 [details] [diff] [review]:
-----------------------------------------------------------------
Comment 8 Craig Topper 2011-05-19 22:41:49 PDT
Created attachment 533891 [details] [diff] [review]
Part 3: Remove nsFrameSelection QI and createInstance support.
Comment 9 Craig Topper 2011-05-19 22:43:41 PDT
Comment on attachment 533891 [details] [diff] [review]
Part 3: Remove nsFrameSelection QI and createInstance support.

Somehow the nsLayoutCID.h change was applied against another patch in my tree. This version fixes that. Carrying forward r+
Comment 11 :Ms2ger 2011-05-20 07:01:16 PDT
I love it. Does nsFrameSelection still need to inherit from nsISupports?
Comment 12 :Ehsan Akhgari (out sick) 2011-05-20 15:00:49 PDT
(In reply to comment #11)
> I love it. Does nsFrameSelection still need to inherit from nsISupports?

I don't think so.
Comment 13 Craig Topper 2011-05-20 18:58:06 PDT
I'll look into that and de-virtualizing its methods. Also going to remove the IID define I forgot to remove.
Comment 14 Craig Topper 2011-05-20 19:16:28 PDT
I think the cycle collecting stuff requires the nsISupports inheritance. :(
Comment 15 Craig Topper 2011-05-20 20:11:40 PDT
nevermind clearly i don't really understand cycle collecting.

Note You need to log in before you can comment on or make changes to this bug.