Bug 640158 - restrict restoring persisted attributes when loading overlays after the first document load, r=bz
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 04 Dec 2013 17:15:52 +0100
changeset 174584 8853618ddaa85eab986d01809e3c2cb695f762ca
parent 174583 05a8fa1c32abd40a09c7dc471dbea61a2544ff13
child 174585 757952431e858d5386ad89fd3bc20c1c1974e1ef
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs640158
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 640158 - restrict restoring persisted attributes when loading overlays after the first document load, r=bz
content/xul/document/src/XULDocument.cpp
content/xul/document/src/XULDocument.h
content/xul/document/test/chrome.ini
content/xul/document/test/overlay_640158.xul
content/xul/document/test/test_bug640158_overlay_persist.xul
--- a/content/xul/document/src/XULDocument.cpp
+++ b/content/xul/document/src/XULDocument.cpp
@@ -212,16 +212,19 @@ XULDocument::XULDocument(void)
 XULDocument::~XULDocument()
 {
     NS_ASSERTION(mNextSrcLoadWaiter == nullptr,
         "unreferenced document still waiting for script source to load?");
 
     // In case we failed somewhere early on and the forward observer
     // decls never got resolved.
     mForwardReferences.Clear();
+    // Likewise for any references we have to IDs where we might
+    // look for persisted data:
+    mPersistenceIds.Clear();
 
     // Destroy our broadcaster map.
     if (mBroadcasterMap) {
         PL_DHashTableDestroy(mBroadcasterMap);
     }
 
     if (mLocalStore) {
         nsCOMPtr<nsIRDFRemoteDataSource> remote =
@@ -2175,16 +2178,21 @@ XULDocument::ApplyPersistentAttributes()
     // model.
     if (!mLocalStore)
         return NS_OK;
 
     mApplyingPersistedAttrs = true;
     ApplyPersistentAttributesInternal();
     mApplyingPersistedAttrs = false;
 
+    // After we've applied persistence once, we should only reapply
+    // it to nodes created by overlays
+    mRestrictPersistence = true;
+    mPersistenceIds.Clear();
+
     return NS_OK;
 }
 
 
 nsresult 
 XULDocument::ApplyPersistentAttributesInternal()
 {
     nsCOMArray<nsIContent> elements;
@@ -2219,16 +2227,19 @@ XULDocument::ApplyPersistentAttributesIn
             continue;
 
         nsAutoString id;
         nsXULContentUtils::MakeElementID(this, nsDependentCString(uri), id);
 
         if (id.IsEmpty())
             continue;
 
+        if (mRestrictPersistence && !mPersistenceIds.Contains(id))
+            continue;
+
         // This will clear the array if there are no elements.
         GetElementsForID(id, elements);
 
         if (!elements.Count())
             continue;
 
         ApplyPersistentAttributesToElements(resource, elements);
     }
@@ -2971,16 +2982,25 @@ XULDocument::ResumeWalk()
                                                     getter_AddRefs(child),
                                                     false);
                     if (NS_FAILED(rv)) return rv;
 
                     // ...and append it to the content model.
                     rv = element->AppendChildTo(child, false);
                     if (NS_FAILED(rv)) return rv;
 
+                    // If we're only restoring persisted things on
+                    // some elements, store the ID here to do that.
+                    if (mRestrictPersistence) {
+                        nsIAtom* id = child->GetID();
+                        if (id) {
+                            mPersistenceIds.PutEntry(nsDependentAtomString(id));
+                        }
+                    }
+
                     // do pre-order document-level hookup, but only if
                     // we're in the master document. For an overlay,
                     // this will happen when the overlay is
                     // successfully resolved.
                     if (mState == eState_Master)
                         AddElementToDocumentPre(child);
                 }
                 else {
--- a/content/xul/document/src/XULDocument.h
+++ b/content/xul/document/src/XULDocument.h
@@ -321,16 +321,22 @@ protected:
      * stylesheet finishes loading while the PD walk is still in
      * progress (waiting for an overlay to finish loading).
      * mStillWalking prevents DoneLoading (and StartLayout) from being
      * called in this situation.
      */
     bool                       mStillWalking;
 
     /**
+     * These two values control where persistent attributes get applied.
+     */
+    bool                           mRestrictPersistence;
+    nsTHashtable<nsStringHashKey>  mPersistenceIds;
+
+    /**
      * An array of style sheets, that will be added (preserving order) to the
      * document after all of them are loaded (in DoneWalking).
      */
     nsTArray<nsRefPtr<nsCSSStyleSheet> > mOverlaySheets;
 
     nsCOMPtr<nsIDOMXULCommandDispatcher>     mCommandDispatcher; // [OWNER] of the focus tracker
 
     // Maintains the template builders that have been attached to
--- a/content/xul/document/test/chrome.ini
+++ b/content/xul/document/test/chrome.ini
@@ -13,9 +13,10 @@ support-files =
 [test_bug403868.xul]
 [test_bug414907.xul]
 [test_bug418216.xul]
 [test_bug445177.xul]
 [test_bug449457.xul]
 [test_bug468176.xul]
 [test_bug497875.xul]
 [test_bug583948.xul]
+[test_bug640158_overlay_persist.xul]
 [test_bug757137.xul]
new file mode 100644
--- /dev/null
+++ b/content/xul/document/test/overlay_640158.xul
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<overlay id="overlay1"
+         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <window id="rootwin">
+    <box id="bar" testattr="original"/>
+  </window>
+</overlay>
+
new file mode 100644
--- /dev/null
+++ b/content/xul/document/test/test_bug640158_overlay_persist.xul
@@ -0,0 +1,51 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=640158
+-->
+<window title="Mozilla Bug 640158" id="rootwin"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=640158"
+     target="_blank">Mozilla Bug 640158</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+
+  SimpleTest.waitForExplicitFinish();
+  window.onload = function onload() {
+    is($("foo").getAttribute("testattr"), "original", "Attribute should be in original state");
+    // Change and persist another value:
+    $("foo").setAttribute("testattr", "changed");
+    document.persist("foo", "testattr");
+    $("foo").setAttribute("testattr", "original");
+
+    // Hacky times: check that items which are overlaid do get persisted into correctly,
+    // by first creating an extra element and persisting the value before loading an
+    // overlay that changes that value - the persisted value should be reinstated.
+    let root = document.documentElement;
+    let bar = document.createElement("box");
+    bar.id = "bar";
+    bar.setAttribute("testattr", "changed"); // The overlay we load has 'original'
+    root.appendChild(bar);
+    document.persist("bar", "testattr");
+    document.loadOverlay(location.href.replace(/[^\\\/]*.xul/, "overlay_bug640158.xul"), function() {
+      is($("foo").getAttribute("testattr"), "original",
+         "Non-overlaid attribute should still be in original state");
+      is($("bar").getAttribute("testattr"), "changed",
+         "Overlaid attribute should have been changed.");
+      SimpleTest.finish();
+    });
+  }
+
+  ]]></script>
+
+  <box id="foo" testattr="original"/>
+
+</window>
+