Bug 449781. Toggling display on a subframe shouldn't give the subframe's document the same device context as the parent document. r=roc, sr=jst
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 18 Aug 2008 15:22:19 -0400
changeset 17009 1ecbdcb04b8e954ea39d8b953f924cc5915fa724
parent 17008 9d17b503e14461410892763d076b8f94dbfee386
child 17010 12bb7139352cd93a40eee281f2dee027f11386a1
push id1339
push userbzbarsky@mozilla.com
push dateMon, 18 Aug 2008 19:23:20 +0000
treeherdermozilla-central@1ecbdcb04b8e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, jst
bugs449781
milestone1.9.1a2pre
Bug 449781. Toggling display on a subframe shouldn't give the subframe's document the same device context as the parent document. r=roc, sr=jst
docshell/base/nsDocShell.cpp
docshell/base/nsIContentViewer.idl
layout/base/nsDocumentViewer.cpp
layout/base/tests/Makefile.in
layout/base/tests/test_bug449781.html
testing/mochitest/tests/SimpleTest/Makefile.in
testing/mochitest/tests/SimpleTest/WindowSnapshot.js
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -187,17 +187,16 @@
 
 #include "nsIFrame.h"
 
 // for embedding
 #include "nsIWebBrowserChromeFocus.h"
 
 #include "nsPluginError.h"
 
-static NS_DEFINE_IID(kDeviceContextCID, NS_DEVICE_CONTEXT_CID);
 static NS_DEFINE_CID(kDOMScriptObjectFactoryCID,
                      NS_DOM_SCRIPT_OBJECT_FACTORY_CID);
 static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID);
 
 #if defined(DEBUG_bryner) || defined(DEBUG_chb)
 //#define DEBUG_DOCSHELL_FOCUS
 #define DEBUG_PAGE_CACHE
 #endif
@@ -6384,26 +6383,19 @@ nsDocShell::SetupNewViewer(nsIContentVie
         mContentViewer = nsnull;
     }
 
     mContentViewer = aNewViewer;
 
     nsCOMPtr<nsIWidget> widget;
     NS_ENSURE_SUCCESS(GetMainWidget(getter_AddRefs(widget)), NS_ERROR_FAILURE);
 
-    nsCOMPtr<nsIDeviceContext> deviceContext;
-    if (widget) {
-        deviceContext = do_CreateInstance(kDeviceContextCID);
-        NS_ENSURE_TRUE(deviceContext, NS_ERROR_FAILURE);
-        deviceContext->Init(widget->GetNativeData(NS_NATIVE_WIDGET));
-    }
-
     nsRect bounds(x, y, cx, cy);
 
-    if (NS_FAILED(mContentViewer->Init(widget, deviceContext, bounds))) {
+    if (NS_FAILED(mContentViewer->Init(widget, bounds))) {
         mContentViewer = nsnull;
         NS_ERROR("ContentViewer Initialization failed");
         return NS_ERROR_FAILURE;
     }
 
     // If we have old state to copy, set the old state onto the new content
     // viewer
     if (newMUDV) {
--- a/docshell/base/nsIContentViewer.idl
+++ b/docshell/base/nsIContentViewer.idl
@@ -2,30 +2,27 @@
 
 interface nsIDOMDocument;
 interface nsISHEntry;
 interface nsIPrintSettings;
 
 
 %{ C++
 class nsIWidget;
-class nsIDeviceContext;
 struct nsRect;
 %}
 
 [ptr] native nsIWidgetPtr(nsIWidget);
-[ptr] native nsIDeviceContextPtr(nsIDeviceContext);
 [ref] native nsRectRef(nsRect);
 
-[scriptable, uuid(89653afe-182f-401f-9f3c-8858d91387cd)]
+[scriptable, uuid(05b290ac-d880-4900-bb1f-2211b5f8accc)]
 interface nsIContentViewer : nsISupports
 {
 
   [noscript] void init(in nsIWidgetPtr aParentWidget,
-                       in nsIDeviceContextPtr aDeviceContext,
                        [const] in nsRectRef aBounds);
 
   attribute nsISupports container;
 
   void loadStart(in nsISupports aDoc);
   void loadComplete(in unsigned long aStatus);
   boolean permitUnload();
   void pageHide(in boolean isUnload);
--- a/layout/base/nsDocumentViewer.cpp
+++ b/layout/base/nsDocumentViewer.cpp
@@ -352,19 +352,27 @@ protected:
 
 private:
   /**
    * Creates a view manager, root view, and widget for the root view, setting
    * mViewManager and mWindow.
    * @param aSize the initial size in appunits
    */
   nsresult MakeWindow(const nsSize& aSize);
+
+  /**
+   * Create our device context
+   */
+  nsresult CreateDeviceContext(nsIWidget* aWidget);
+
+  /**
+   * Make sure to set up mDeviceContext as needed before calling this
+   */
   nsresult InitInternal(nsIWidget* aParentWidget,
                         nsISupports *aState,
-                        nsIDeviceContext* aDeviceContext,
                         const nsRect& aBounds,
                         PRBool aDoCreation,
                         PRBool aInPrintPreview,
                         PRBool aNeedMakeCX = PR_TRUE);
   /**
    * @param aDoInitialReflow set to true if you want to kick off the initial
    * reflow
    * @param aReenableRefresh set to true if you want this to reenable refresh
@@ -402,17 +410,17 @@ protected:
 
   // IMPORTANT: The ownership implicit in the following member
   // variables has been explicitly checked and set using nsCOMPtr
   // for owning pointers and raw COM interface pointers for weak
   // (ie, non owning) references. If you add any members to this
   // class, please make the ownership explicit (pinkerton, scc).
 
   nsWeakPtr mContainer; // it owns me!
-  nsCOMPtr<nsIDeviceContext> mDeviceContext;   // ??? can't hurt, but...
+  nsCOMPtr<nsIDeviceContext> mDeviceContext;  // We create and own this baby
 
   // the following six items are explicitly in this order
   // so they will be destroyed in the reverse order (pinkerton, scc)
   nsCOMPtr<nsIDocument>    mDocument;
   nsCOMPtr<nsIWidget>      mWindow;      // ??? should we really own it?
   nsCOMPtr<nsIViewManager> mViewManager;
   nsCOMPtr<nsPresContext> mPresContext;
   nsCOMPtr<nsIPresShell>   mPresShell;
@@ -475,16 +483,17 @@ protected:
 };
 
 //------------------------------------------------------------------
 // DocumentViewerImpl
 //------------------------------------------------------------------
 // Class IDs
 static NS_DEFINE_CID(kViewManagerCID,       NS_VIEW_MANAGER_CID);
 static NS_DEFINE_CID(kWidgetCID,            NS_CHILD_CID);
+static NS_DEFINE_CID(kDeviceContextCID,     NS_DEVICE_CONTEXT_CID);
 
 //------------------------------------------------------------------
 nsresult
 NS_NewDocumentViewer(nsIDocumentViewer** aResult)
 {
   *aResult = new DocumentViewerImpl();
   if (!*aResult) {
     return NS_ERROR_OUT_OF_MEMORY;
@@ -652,20 +661,22 @@ DocumentViewerImpl::GetContainer(nsISupp
    *aResult = nsnull;
    nsCOMPtr<nsISupports> container = do_QueryReferent(mContainer);
    container.swap(*aResult);
    return NS_OK;
 }
 
 NS_IMETHODIMP
 DocumentViewerImpl::Init(nsIWidget* aParentWidget,
-                         nsIDeviceContext* aDeviceContext,
                          const nsRect& aBounds)
 {
-  return InitInternal(aParentWidget, nsnull, aDeviceContext, aBounds, PR_TRUE, PR_FALSE);
+  nsresult rv = CreateDeviceContext(aParentWidget);
+  NS_ENSURE_SUCCESS(rv, rv);
+  
+  return InitInternal(aParentWidget, nsnull, aBounds, PR_TRUE, PR_FALSE);
 }
 
 nsresult
 DocumentViewerImpl::InitPresentationStuff(PRBool aDoInitialReflow, PRBool aReenableRefresh)
 {
   // Create the style set...
   nsStyleSet *styleSet;
   nsresult rv = CreateStyleSet(mDocument, &styleSet);
@@ -789,42 +800,39 @@ DocumentViewerImpl::InitPresentationStuf
 
 //-----------------------------------------------
 // This method can be used to initial the "presentation"
 // The aDoCreation indicates whether it should create
 // all the new objects or just initialize the existing ones
 nsresult
 DocumentViewerImpl::InitInternal(nsIWidget* aParentWidget,
                                  nsISupports *aState,
-                                 nsIDeviceContext* aDeviceContext,
                                  const nsRect& aBounds,
                                  PRBool aDoCreation,
                                  PRBool aInPrintPreview,
                                  PRBool aNeedMakeCX /*= PR_TRUE*/)
 {
   mParentWidget = aParentWidget; // not ref counted
 
   nsresult rv = NS_OK;
   NS_ENSURE_TRUE(mDocument, NS_ERROR_NULL_POINTER);
 
-  mDeviceContext = aDeviceContext;
-
   PRBool makeCX = PR_FALSE;
   if (aDoCreation) {
     if (aParentWidget && !mPresContext) {
       // Create presentation context
       if (mIsPageMode) {
         //Presentation context already created in SetPageMode which is calling this method
       }
       else
         mPresContext =
             new nsPresContext(mDocument, nsPresContext::eContext_Galley);
       NS_ENSURE_TRUE(mPresContext, NS_ERROR_OUT_OF_MEMORY);
 
-      nsresult rv = mPresContext->Init(aDeviceContext); 
+      nsresult rv = mPresContext->Init(mDeviceContext); 
       if (NS_FAILED(rv)) {
         mPresContext = nsnull;
         return rv;
       }
 
 #if defined(NS_PRINTING) && defined(NS_PRINT_PREVIEW)
       makeCX = !GetIsPrintPreview() && aNeedMakeCX; // needs to be true except when we are already in PP or we are enabling/disabling paginated mode.
 #else
@@ -1237,18 +1245,17 @@ DocumentViewerImpl::Open(nsISupports *aS
   // If this is the case, we must fail to open so we don't crash.
   nsCOMPtr<nsISupports> container = do_QueryReferent(mContainer);
   if (!container)
     return NS_ERROR_NOT_AVAILABLE;
 
   nsRect bounds;
   mWindow->GetBounds(bounds);
 
-  nsresult rv = InitInternal(mParentWidget, aState, mDeviceContext, bounds,
-                             PR_FALSE, PR_FALSE);
+  nsresult rv = InitInternal(mParentWidget, aState, bounds, PR_FALSE, PR_FALSE);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (mDocument)
     mDocument->SetContainer(nsCOMPtr<nsISupports>(do_QueryReferent(mContainer)));
 
   if (mPresShell)
     mPresShell->SetForwardingContainer(nsnull);
 
@@ -1867,26 +1874,25 @@ DocumentViewerImpl::Show(void)
     }
   }
 
   if (mWindow) {
     mWindow->Show(PR_TRUE);
   }
 
   if (mDocument && !mPresShell && !mWindow) {
-    nsresult rv;
-
     nsCOMPtr<nsIBaseWindow> base_win(do_QueryReferent(mContainer));
     NS_ENSURE_TRUE(base_win, NS_ERROR_UNEXPECTED);
 
     base_win->GetParentWidget(&mParentWidget);
     NS_ENSURE_TRUE(mParentWidget, NS_ERROR_UNEXPECTED);
     mParentWidget->Release(); // GetParentWidget AddRefs, but mParentWidget is weak
 
-    mDeviceContext = mParentWidget->GetDeviceContext();
+    nsresult rv = CreateDeviceContext(mParentWidget);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     // Create presentation context
     NS_ASSERTION(!mPresContext, "Shouldn't have a prescontext if we have no shell!");
     mPresContext = new nsPresContext(mDocument, nsPresContext::eContext_Galley);
     NS_ENSURE_TRUE(mPresContext, NS_ERROR_OUT_OF_MEMORY);
 
     rv = mPresContext->Init(mDeviceContext);
     if (NS_FAILED(rv)) {
@@ -2266,16 +2272,28 @@ DocumentViewerImpl::MakeWindow(const nsS
   // This SetFocus is necessary so the Arrow Key and Page Key events
   // go to the scrolled view as soon as the Window is created instead of going to
   // the browser window (this enables keyboard scrolling of the document)
   // mWindow->SetFocus();
 
   return rv;
 }
 
+nsresult
+DocumentViewerImpl::CreateDeviceContext(nsIWidget* aWidget)
+{
+  NS_PRECONDITION(!mDeviceContext, "How come we're calling this?");
+  if (aWidget) {
+    mDeviceContext = do_CreateInstance(kDeviceContextCID);
+    NS_ENSURE_TRUE(mDeviceContext, NS_ERROR_FAILURE);
+    mDeviceContext->Init(aWidget->GetNativeData(NS_NATIVE_WIDGET));
+  }
+  return NS_OK;
+}
+
 // Return the selection for the document. Note that text fields have their
 // own selection, which cannot be accessed with this method. Use
 // mPresShell->GetSelectionForCopy() instead.
 nsresult DocumentViewerImpl::GetDocumentSelection(nsISelection **aSelection)
 {
   NS_ENSURE_ARG_POINTER(aSelection);
   if (!mPresShell) {
     return NS_ERROR_NOT_INITIALIZED;
@@ -4100,17 +4118,17 @@ NS_IMETHODIMP DocumentViewerImpl::SetPag
     mPresContext =
       new nsPresContext(mDocument, nsPresContext::eContext_PageLayout);
     NS_ENSURE_TRUE(mPresContext, NS_ERROR_OUT_OF_MEMORY);
     mPresContext->SetPaginatedScrolling(PR_TRUE);
     mPresContext->SetPrintSettings(aPrintSettings);
     nsresult rv = mPresContext->Init(mDeviceContext);
     NS_ENSURE_SUCCESS(rv, rv);
   }
-  InitInternal(mParentWidget, nsnull, mDeviceContext, bounds, PR_TRUE, PR_FALSE, PR_FALSE);
+  InitInternal(mParentWidget, nsnull, bounds, PR_TRUE, PR_FALSE, PR_FALSE);
   mViewManager->EnableRefresh(NS_VMREFRESH_NO_SYNC);
 
   Show();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 DocumentViewerImpl::GetHistoryEntry(nsISHEntry **aHistoryEntry)
--- a/layout/base/tests/Makefile.in
+++ b/layout/base/tests/Makefile.in
@@ -88,16 +88,17 @@ ifdef MOZ_MOCHITEST
 		test_bug396024.html \
 		test_bug399284.html \
 		test_bug399951.html \
 		test_bug404209.xhtml \
 		test_bug416896.html \
 		test_bug420499.xul \
 		test_bug423523.html \
 		test_bug445810.html \
+		test_bug449781.html \
 		$(NULL)
 # test_bug396024.html is currently disabled because it interacts badly with
 # the "You can't print-preview while the page is loading" dialog.
 # (See bug 407080)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
 
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/test_bug449781.html
@@ -0,0 +1,71 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=449781
+-->
+<head>
+  <title>Test for Bug 449781</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/WindowSnapshot.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=449781">Mozilla Bug 449781</a>
+<p id="display">Canary</p>
+<iframe src="about:blank" id="ourFrame" style="visibility: hidden">
+</iframe>
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+var s1, s2, s3, s4;
+
+/** Test for Bug 449781 **/
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(function() {
+  s1 = snapshotWindow(window);
+
+  $("ourFrame").style.display = "none";
+  is($("ourFrame").offsetWidth, 0, "Unexpected width after hiding");
+  $("ourFrame").style.display = "";
+  is($("ourFrame").clientWidth, 300, "Unexpected width after showing");
+
+  s2 = snapshotWindow(window);
+
+  var equal, str1, str2;
+  [equal, str1, str2] = compareSnapshots(s1, s2);
+  ok(equal, "Show/hide should have no effect",
+     "got " + str1 + " but expected " + str2);
+
+  netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+  var viewer =
+    $("ourFrame").contentWindow
+                 .QueryInterface(Components.interfaces.nsIInterfaceRequestor)
+                 .getInterface(Components.interfaces.nsIWebNavigation)
+                 .QueryInterface(Components.interfaces.nsIDocShell)
+                 .contentViewer
+                 .QueryInterface(Components.interfaces.nsIMarkupDocumentViewer);
+  viewer.fullZoom = 2;
+  
+  s3 = snapshotWindow(window);
+
+  [equal, str1, str2] = compareSnapshots(s1, s3);
+  ok(equal, "Zoom should have no effect",
+     "got " + str1 + " but expected " + str2);
+
+  $("display").style.display = "none";
+
+  s4 = snapshotWindow(window);
+  [equal, str1, str2] = compareSnapshots(s3, s4);
+  ok(!equal, "Should be able to see the canary");
+
+  SimpleTest.finish();
+});
+
+
+
+</script>
+</pre>
+</body>
+</html>
+
--- a/testing/mochitest/tests/SimpleTest/Makefile.in
+++ b/testing/mochitest/tests/SimpleTest/Makefile.in
@@ -46,13 +46,14 @@ include $(topsrcdir)/config/rules.mk
 
 _SIMPLETEST_FILES =	MozillaFileLogger.js \
 			quit.js \
 			SimpleTest.js \
 			test.css \
 			TestRunner.js \
 			setup.js \
 			EventUtils.js \
+			WindowSnapshot.js \
 			$(NULL)
 
 libs:: $(_SIMPLETEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/$(relativesrcdir)
 
new file mode 100644
--- /dev/null
+++ b/testing/mochitest/tests/SimpleTest/WindowSnapshot.js
@@ -0,0 +1,47 @@
+netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+var gWindowSnapshotCompareHelper;
+
+try {
+  gWindowSnapshotCompareHelper =
+    Components.classes["@mozilla.org/reftest-helper;1"]
+              .getService(Components.interfaces.nsIReftestHelper);
+} catch (e) {
+  gWindowSnapshotCompareHelper = null;
+}
+
+function snapshotWindow(win) {
+  var el = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas");
+  el.width = win.innerWidth;
+  el.height = win.innerHeight;
+
+  // drawWindow requires privileges
+  netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+  
+  el.getContext("2d").drawWindow(win, win.scrollX, win.scrollY,
+				 win.innerWidth, win.innerHeight,
+				 "rgb(255,255,255)");
+  return el;
+}
+
+// If the two snapshots aren't equal, returns their serializations as data URIs.
+function compareSnapshots(s1, s2) {
+  netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+  var s1Str, s2Str;
+  var equal = false;
+  if (gWindowSnapshotCompareHelper) {
+    equal = (gWindowSnapshotCompareHelper.compareCanvas(s1, s2) == 0);
+  }
+
+  if (!equal) {
+    s1Str = s1.toDataURL();
+    s2Str = s2.toDataURL();
+
+    if (!gWindowSnapshotCompareHelper) {
+      equal = (s1Str == s2Str);
+    }
+  }
+
+  return [equal, s1Str, s2Str];
+}