232731 - Minor cleanup in documentviewer/printing code

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Harshal Pradhan]

A few minor things accumulated in my tree while hunting some other bug. Ought to
be cleaned up...

- nsDocumentViewer::IsWebShellAFrameSet() is unused and can be removed.
- FindFrameSetWithIID() and GetPresShellAndRootContent() on the 
  nsIDocumentViewerPrint interface make no sense at all since they have
  nothing to do with the DV. They can just be replaced by static helpers
  in nsPrintEngine, which is the only thing that uses them. 
- GetWebShellAndRootContent() only uses the document and doesnt even
   touch its webshell param. And it can be static.
- GetPresShell() in nsPrintEngine is unused.

Patch coming up.

Comment 1

[Harshal Pradhan]

Attachment: Fix

Comment 2

[Harshal Pradhan]

Comment on attachment 140299 [details] [diff] [review]
Fix

jst, r+sr ?

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 140299 [details] [diff] [review]
Fix

- In nsPrintEngine::GetPresShellAndRootContent():

+  nsCOMPtr<nsIDOMDocument> domDoc(do_GetInterface(aDocShell));
+  if (!domDoc)
+    return;
+
+  nsCOMPtr<nsIDocument> doc(do_QueryInterface(domDoc));
+  if (!doc)
+    return;

You can loose the first null-check, do_QueryInterface() is null safe.

- In nsPrintEngine::HasFramesetChild():

+  nsCOMPtr<nsIDOMHTMLFrameSetElement> temp;
+  for (PRUint32 i = 0; i < numChildren; ++i) {
+    nsIContent *child = aContent->GetChildAt(i);
+    if (child) {

No need to check here, child won't be null.

+      temp = do_QueryInterface(child);
+      if (temp) {
+	 return PR_TRUE;

I think this would be faster as:

+      if (child->Tag() == nsHTMLAtoms::frameset &&
+	   child->IsContentOfType(nsIContent::eHTML)) {
+	 return PR_TRUE;

r+sr=jst

Comment 4

[Harshal Pradhan]

Attachment: Final patch for checkin

Thanks for the review. This patch addresses all those comments.

Comment 5

[Harshal Pradhan]

Checked in.