As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 540433 - Remove nsIDocumentViewer
: Remove nsIDocumentViewer
Status: RESOLVED FIXED
: addon-compat, dev-doc-complete
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla10
Assigned To: :Ms2ger (⌚ UTC+1/+2)
:
:
Mentors:
Depends on: 540462 694749
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-18 07:48 PST by Olli Pettay [:smaug] (review queue closed until backlog cleared)
Modified: 2011-12-14 07:01 PST (History)
8 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (62.39 KB, patch)
2011-10-12 12:04 PDT, :Ms2ger (⌚ UTC+1/+2)
bugs: review+
Details | Diff | Splinter Review

Description User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2010-01-18 07:48:06 PST
I think we could merge nsIDocumentViewer and nsIContentViewer
together.
Most common case to use nsIDocumentViewer is to get nsIDocument from it, 
but nsIContentViewer has already attribute DOMDocument.
For performance reasons I'd add [noscript,notxpcom] nsIDocumentPtr getDocument();
to nsIContenViewer and remove NS_IMETHOD GetDocument(nsIDocument** aResult)
from nsIDocumentViewer.

After that removing the remaining methods GetPresShell(nsIPresShell** aResult)
GetPresContext(nsPresContext** aResult) should be easy.

(Noticed this all while profiling nsDocShell::GetChannelIsUnsafe)

Any objections? Suggestions?
Comment 1 User image Boris Zbarsky [:bz] (still a bit busy) 2010-01-18 11:16:36 PST
I'm all in favor of reducing the number of interfaces dealing with this stuff.  I think the theory was that we could have non-document viewers (e.g. for plug-ins or images PDFs or whatnot), but I think that's a silly theory.  We should just keep using synthetic documents.
Comment 2 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2010-01-20 12:44:24 PST
Absolutely. Let's do it!
Comment 3 User image :Ms2ger (⌚ UTC+1/+2) 2011-10-12 12:04:32 PDT
Created attachment 566602 [details] [diff] [review]
Patch v1
Comment 4 User image :Ms2ger (⌚ UTC+1/+2) 2011-10-12 12:07:50 PDT
I should probably rename NS_NewDocumentViewer as well, and I'll rev the iid.
Comment 5 User image Olli Pettay [:smaug] (review queue closed until backlog cleared) 2011-10-12 13:35:46 PDT
Comment on attachment 566602 [details] [diff] [review]
Patch v1


>-NS_NewDocumentViewer(nsIDocumentViewer** aResult);
>+NS_NewDocumentViewer(nsIContentViewer** aResult);
Rename the method.


> MAKE_CTOR(CreateNameSpaceManager,         nsINameSpaceManager,         NS_GetNameSpaceManager)
>-MAKE_CTOR(CreateDocumentViewer,           nsIDocumentViewer,           NS_NewDocumentViewer)
>+MAKE_CTOR(CreateDocumentViewer,           nsIContentViewer,            NS_NewDocumentViewer)
CreateContentViewer   ... NS_NewContentViewer



>     if (mIsCreatingPrintPreview) {
>-      nsCOMPtr<nsIDocumentViewer> dv = do_QueryInterface(mDocViewerPrint);
>-      if (dv) {
>-        parentView = dv->FindContainerView();
>+      nsCOMPtr<nsIContentViewer> cv = do_QueryInterface(mDocViewerPrint);
>+      if (cv) {
>+        parentView = cv->FindContainerView();
>       }
Could you file a followup to rename mDocViewerPrint and nsIDocumentViewerPrint
(if I haven't filed that already)
Comment 6 User image :Ms2ger (⌚ UTC+1/+2) 2011-10-15 02:59:03 PDT
https://hg.mozilla.org/mozilla-central/rev/e915987a1cda
Comment 7 User image Takanori MATSUURA 2011-10-15 03:16:13 PDT
The fix has been checked in by http://hg.mozilla.org/mozilla-central/rev/e915987a1cda

However nsIDocumentViewer.h is required by following files in comm-central.
/mailnews/base/src/nsMsgPrintEngine.cpp
/mailnews/base/src/nsMsgStatusFeedback.cpp
Comment 8 User image Eric Shepherd [:sheppy] 2011-12-14 07:01:27 PST
This is mentioned on Firefox 10 for developers. In addition, I've added this new article:

https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIContentViewer

Neither nsIContentViewer nor nsIDocumentViewer had documentation previously.

That article is not complete but is a start. If anyone wants to fill out the rest, awesome. But for the purposes of this particular bug, it's done.

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