Closed Bug 409331 Opened 14 years ago Closed 14 years ago

convert sites that QueryInterface to kBlockCID to use nsLayoutUtils::GetAsBlock

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: karunasagark)

Details

(Whiteboard: good first bug)

Attachments

(1 file, 2 obsolete files)

nsLayoutUtils::GetAsBlock provides a cleaner interface than using QueryInterface. 

Some sites that test frame->GetType() == nsGkAtoms::blockFrame/areaFrame should also use GetAsBlock instead and test for null.
Whiteboard: good first bug
working on this ...
Replacing the checks frame->GetType() == nsGkAtoms::blockFrame and frame->GetType() == nsGkAtoms::areaFrame seems unnecessary and would be asymmetrical. Any reason why we need to do this replacement??
I'm not sure what you mean by "asymmetrical". Doing this replacement where only blockFrame and areaFrame are being checked for (or when only blockFrame is being checked for but areaFrame *should* be checked for) should result in smaller simpler code in the caller.
+  block = nsLayoutUtils::GetAsBlock(aBlockFrame);
+  NS_ASSERTION(block,
                "Not a block frame?");

You can just eliminate "block" and write NS_ASSERTION(nsLayoutUtils::GetAsBlock(aBlockFrame), ...);

     nsBlockFrame* blockParent;
-    if (NS_SUCCEEDED(parent->QueryInterface(kBlockFrameCID, (void**)&blockParent)))
+    blockParent = nsLayoutUtils::GetAsBlock(parent);

Put the assignment in the same statement as the declaration,
     nsBlockFrame* blockParent = nsLayoutUtils::GetAsBlock(parent);

       void* bf;
-      if (NS_SUCCEEDED(f->QueryInterface(kBlockFrameCID, &bf))) {
+      bf = static_cast<void*>(nsLayoutUtils::GetAsBlock(f));
+      if (bf) {
         MarkAllDescendantLinesDirty(static_cast<nsBlockFrame*>(f));

As above, and also, make bf an nsBlockFrame* and remove these unnecessary casts. (Happens in lots of places)
Attachment #294935 - Attachment is obsolete: true
Attachment #294947 - Flags: review?(roc)
Attachment #294935 - Flags: review?(roc)
 #ifdef DEBUG
-  nsBlockFrame* block;
-  NS_ASSERTION(NS_SUCCEEDED(aBlockFrame->QueryInterface(kBlockFrameCID,
-                                                        (void**)&block)) &&
-               block,
+  NS_ASSERTION(nsLayoutUtils::GetAsBlock(aBlockFrame),
                "Not a block frame?");
 #endif

You can remove #ifdef DEBUG because NS_ASSERTION does nothing in a non-DEBUG build.

 #ifdef DEBUG
   {
-    nsBlockFrame* block;
-    NS_ASSERTION(NS_SUCCEEDED(aBlockFrame->QueryInterface(kBlockFrameCID,
-                                                          (void**)&block)) &&
-                 block,
+    NS_ASSERTION(nsLayoutUtils::GetAsBlock(aBlockFrame),
                  "Not a block frame?");
   }
 #endif

Ditto

+      nsBlockFrame* bf =  nsLayoutUtils::GetAsBlock(f);

Remove extra space

+      if (bf) {
         MarkAllDescendantLinesDirty(static_cast<nsBlockFrame*>(f));

Use bf instead of the cast

     blockWithSpaceMgr = static_cast<nsBlockFrame*>(blockWithSpaceMgr->GetParent());

Ditto

   nsBlockFrame* block = static_cast<nsBlockFrame*>(aFrame);

Ditto

 #ifdef DEBUG
-  nsBlockFrame* blockFrame;
-  aBlock->QueryInterface(kBlockFrameCID, (void**)&blockFrame);
-  NS_ASSERTION(blockFrame, "aBlock must be a block");
+  NS_ASSERTION(nsLayoutUtils::GetAsBlock(aBlock), "aBlock must be a block");
 #endif

Remove unnecessary #ifdef DEBUG

 #ifdef DEBUG
-  nsBlockFrame* blockFrame;
-  aBlock->QueryInterface(kBlockFrameCID, (void**)&blockFrame);
-  NS_ASSERTION(blockFrame, "aBlock must be a block");
+  NS_ASSERTION(nsLayoutUtils::GetAsBlock(aBlock), "aBlock must be a block");
 #endif

Ditto

+      reachedBlockAncestor = (nsLayoutUtils::GetAsBlock(frame)) ? PR_TRUE : PR_FALSE;

Make this nsLayoutUtils::GetAsBlock(frame) != nsnull

+      reachedBlockAncestor = (nsLayoutUtils::GetAsBlock(frame)) ? PR_TRUE : PR_FALSE;

Ditto
Attachment #294947 - Attachment is obsolete: true
Attachment #295022 - Flags: review?(roc)
Attachment #294947 - Flags: review?(roc)
Comment on attachment 295022 [details] [diff] [review]
convert sites that QueryInterface to kBlockCID to use nsLayoutUtils::GetAsBlock

cool.

We probably don't need to take this for FF3 though. Let's land it post-FF3.
Attachment #295022 - Flags: superreview+
Attachment #295022 - Flags: review?(roc)
Attachment #295022 - Flags: review+
Assignee: nobody → karunasagark
OS: Mac OS X → All
Hardware: PC → All
Status: NEW → ASSIGNED
Hmm, I need to land this.
Pushed 140ebc8ba146.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.