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.

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)

#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

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.

Hmm, I need to land this.

Pushed 140ebc8ba146.