Bug 1476678 - Remove document.persist from XULDocument;r=bz draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Sat, 21 Jul 2018 10:46:25 -0700
changeset 821172 b0fdfbdab204f787349b3284fc8c73c4aa799d66
parent 821164 9daa53881b7ae80bf6b093dac5d7744cf7fd18b1
push id117028
push userbgrinstead@mozilla.com
push dateSat, 21 Jul 2018 20:23:34 +0000
reviewersbz
bugs1476678
milestone63.0a1
Bug 1476678 - Remove document.persist from XULDocument;r=bz Also remove overload for XULDocument::Persist and the implementation of XULDocument::DoPersist, since there's only one remaining caller. MozReview-Commit-ID: CPaTdFJ4GOx
dom/webidl/XULDocument.webidl
dom/xul/XULDocument.cpp
dom/xul/XULDocument.h
--- a/dom/webidl/XULDocument.webidl
+++ b/dom/webidl/XULDocument.webidl
@@ -34,13 +34,10 @@ interface XULDocument : Document {
 
   [Throws]
   void addBroadcastListenerFor(Element broadcaster, Element observer,
                                DOMString attr);
   void removeBroadcastListenerFor(Element broadcaster, Element observer,
                                   DOMString attr);
 
   [Throws]
-  void persist([TreatNullAs=EmptyString] DOMString id, DOMString attr);
-
-  [Throws]
   BoxObject? getBoxObjectFor(Element? element);
 };
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -825,19 +825,19 @@ XULDocument::AttributeChanged(Element* a
     nsAutoString persist;
     aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::persist, persist);
     // Persistence of attributes of xul:window is handled in nsXULWindow.
     if (ShouldPersistAttribute(aElement, aAttribute) && !persist.IsEmpty() &&
         // XXXldb This should check that it's a token, not just a substring.
         persist.Find(nsDependentAtomString(aAttribute)) >= 0) {
       nsContentUtils::AddScriptRunner(
         NewRunnableMethod<Element*, int32_t, nsAtom*>(
-          "dom::XULDocument::DoPersist",
+          "dom::XULDocument::Persist",
           this,
-          &XULDocument::DoPersist,
+          &XULDocument::Persist,
           aElement,
           kNameSpaceID_None,
           aAttribute));
     }
 }
 
 void
 XULDocument::ContentAppended(nsIContent* aFirstNewContent)
@@ -994,117 +994,64 @@ XULDocument::GetElementsByAttributeNS(co
                                             attrValue.forget(),
                                             true,
                                             attrAtom,
                                             nameSpaceId);
     return list.forget();
 }
 
 void
-XULDocument::Persist(const nsAString& aID,
-                     const nsAString& aAttr,
-                     ErrorResult& aRv)
-{
-    // If we're currently reading persisted attributes out of the
-    // localstore, _don't_ re-enter and try to set them again!
-    if (mApplyingPersistedAttrs) {
-        return;
-    }
-
-    Element* element = nsDocument::GetElementById(aID);
-    if (!element) {
-        return;
-    }
-
-    RefPtr<nsAtom> tag;
-    int32_t nameSpaceID;
-
-    RefPtr<mozilla::dom::NodeInfo> ni = element->GetExistingAttrNameFromQName(aAttr);
-    nsresult rv;
-    if (ni) {
-        tag = ni->NameAtom();
-        nameSpaceID = ni->NamespaceID();
-    }
-    else {
-        // Make sure that this QName is going to be valid.
-        const char16_t *colon;
-        rv = nsContentUtils::CheckQName(PromiseFlatString(aAttr), true, &colon);
-
-        if (NS_FAILED(rv)) {
-            // There was an invalid character or it was malformed.
-            aRv.Throw(NS_ERROR_INVALID_ARG);
-            return;
-        }
-
-        if (colon) {
-            // We don't really handle namespace qualifiers in attribute names.
-            aRv.Throw(NS_ERROR_NOT_IMPLEMENTED);
-            return;
-        }
-
-        tag = NS_Atomize(aAttr);
-        if (NS_WARN_IF(!tag)) {
-            aRv.Throw(NS_ERROR_OUT_OF_MEMORY);
-            return;
-        }
-
-        nameSpaceID = kNameSpaceID_None;
-    }
-
-    aRv = Persist(element, nameSpaceID, tag);
-}
-
-nsresult
 XULDocument::Persist(Element* aElement, int32_t aNameSpaceID,
                      nsAtom* aAttribute)
 {
     // For non-chrome documents, persistance is simply broken
     if (!nsContentUtils::IsSystemPrincipal(NodePrincipal()))
-        return NS_ERROR_NOT_AVAILABLE;
+        return;
 
     if (!mLocalStore) {
         mLocalStore = do_GetService("@mozilla.org/xul/xulstore;1");
         if (NS_WARN_IF(!mLocalStore)) {
-            return NS_ERROR_NOT_INITIALIZED;
+            return;
         }
     }
 
     nsAutoString id;
 
     aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::id, id);
     nsAtomString attrstr(aAttribute);
 
     nsAutoString valuestr;
     aElement->GetAttr(kNameSpaceID_None, aAttribute, valuestr);
 
     nsAutoCString utf8uri;
     nsresult rv = mDocumentURI->GetSpec(utf8uri);
     if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
+        return;
     }
     NS_ConvertUTF8toUTF16 uri(utf8uri);
 
     bool hasAttr;
     rv = mLocalStore->HasValue(uri, id, attrstr, &hasAttr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
+        return;
     }
 
     if (hasAttr && valuestr.IsEmpty()) {
-        return mLocalStore->RemoveValue(uri, id, attrstr);
+        mLocalStore->RemoveValue(uri, id, attrstr);
+        return;
     }
 
     // Persisting attributes to top level windows is handled by nsXULWindow.
     if (aElement->IsXULElement(nsGkAtoms::window)) {
         if (nsCOMPtr<nsIXULWindow> win = GetXULWindowIfToplevelChrome()) {
-           return NS_OK;
+           return;
         }
     }
 
-    return mLocalStore->SetValue(uri, id, attrstr, valuestr);
+    mLocalStore->SetValue(uri, id, attrstr, valuestr);
 }
 
 static JSObject*
 GetScopeObjectOfNode(nsINode* node)
 {
     MOZ_ASSERT(node, "Must not be called with null.");
 
     // Window root occasionally keeps alive a node of a document whose
--- a/dom/xul/XULDocument.h
+++ b/dom/xul/XULDocument.h
@@ -160,18 +160,16 @@ public:
       GetElementsByAttributeNS(const nsAString& aNamespaceURI,
                                const nsAString& aAttribute,
                                const nsAString& aValue,
                                ErrorResult& aRv);
     void AddBroadcastListenerFor(Element& aBroadcaster, Element& aListener,
                                  const nsAString& aAttr, ErrorResult& aRv);
     void RemoveBroadcastListenerFor(Element& aBroadcaster, Element& aListener,
                                     const nsAString& aAttr);
-    void Persist(const nsAString& aId, const nsAString& aAttr,
-                 ErrorResult& aRv);
     using nsDocument::GetBoxObjectFor;
 
 protected:
     virtual ~XULDocument();
 
     // Implementation methods
     friend nsresult
     (::NS_NewXULDocument(nsIDocument** aResult));
@@ -211,28 +209,20 @@ protected:
 
     static void DirectionChanged(const char* aPrefName, XULDocument* aData);
 
     // pseudo constants
     static int32_t gRefCnt;
 
     static LazyLogModule gXULLog;
 
-    nsresult
+    void
     Persist(mozilla::dom::Element* aElement,
             int32_t aNameSpaceID,
             nsAtom* aAttribute);
-    // Just like Persist but ignores the return value so we can use it
-    // as a runnable method.
-    void DoPersist(mozilla::dom::Element* aElement,
-                   int32_t aNameSpaceID,
-                   nsAtom* aAttribute)
-    {
-        Persist(aElement, aNameSpaceID, aAttribute);
-    }
 
     virtual JSObject* WrapNode(JSContext *aCx, JS::Handle<JSObject*> aGivenProto) override;
 
     // 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).