From 58213a9d14a78980960ac342c49b21a7f1d76fe4 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Tue, 16 Oct 2012 15:37:26 +0300 Subject: [PATCH 1/9] Bug 801609 - Simplify popup and contextmenu handling and make the listener skippable, r=mccr8 --HG-- rename : content/canvas/test/test_toBlob.html => content/canvas/test/test_mozGetAsFile.html rename : dom/base/test/test_gsp-qualified.html => dom/base/test/test_gsp-standards.html extra : rebase_source : 8e017d2fa6ff12cfcf0b0f8a56dadd541aca16bf --- content/base/src/FragmentOrElement.cpp | 10 --- content/base/src/nsGkAtomList.h | 2 - content/xul/content/src/nsXULElement.cpp | 46 ++---------- content/xul/content/src/nsXULElement.h | 8 +- .../xul/content/src/nsXULPopupListener.cpp | 74 +++++++++++-------- content/xul/content/src/nsXULPopupListener.h | 8 +- 6 files changed, 58 insertions(+), 90 deletions(-) diff --git a/content/base/src/FragmentOrElement.cpp b/content/base/src/FragmentOrElement.cpp index bc2ec9256850..5f6e888613aa 100644 --- a/content/base/src/FragmentOrElement.cpp +++ b/content/base/src/FragmentOrElement.cpp @@ -1165,9 +1165,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(FragmentOrElement) tmp->DeleteProperty(nsGkAtoms::itemtype); tmp->DeleteProperty(nsGkAtoms::itemref); tmp->DeleteProperty(nsGkAtoms::itemprop); - } else if (tmp->IsXUL()) { - tmp->DeleteProperty(nsGkAtoms::contextmenulistener); - tmp->DeleteProperty(nsGkAtoms::popuplistener); } } @@ -1708,13 +1705,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(FragmentOrElement) cb.NoteXPCOMChild(property); property = static_cast(tmp->GetProperty(nsGkAtoms::itemtype)); cb.NoteXPCOMChild(property); - } else if (tmp->IsXUL()) { - nsISupports* property = static_cast - (tmp->GetProperty(nsGkAtoms::contextmenulistener)); - cb.NoteXPCOMChild(property); - property = static_cast - (tmp->GetProperty(nsGkAtoms::popuplistener)); - cb.NoteXPCOMChild(property); } } diff --git a/content/base/src/nsGkAtomList.h b/content/base/src/nsGkAtomList.h index 2c6ecf8cb93a..9cf9d22e2b3c 100644 --- a/content/base/src/nsGkAtomList.h +++ b/content/base/src/nsGkAtomList.h @@ -226,7 +226,6 @@ GK_ATOM(headerContentStyleType, "content-style-type") GK_ATOM(headerContentType, "content-type") GK_ATOM(context, "context") GK_ATOM(contextmenu, "contextmenu") -GK_ATOM(contextmenulistener, "contextmenulistener") GK_ATOM(control, "control") GK_ATOM(controls, "controls") GK_ATOM(coords, "coords") @@ -833,7 +832,6 @@ GK_ATOM(popupanchor, "popupanchor") GK_ATOM(popupgroup, "popupgroup") GK_ATOM(popuphidden, "popuphidden") GK_ATOM(popuphiding, "popuphiding") -GK_ATOM(popuplistener, "popuplistener") GK_ATOM(popupset, "popupset") GK_ATOM(popupshowing, "popupshowing") GK_ATOM(popupshown, "popupshown") diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp index 83c866451816..20c33548a62a 100644 --- a/content/xul/content/src/nsXULElement.cpp +++ b/content/xul/content/src/nsXULElement.cpp @@ -1554,58 +1554,26 @@ nsXULElement::IsNodeOfType(uint32_t aFlags) const return !(aFlags & ~eCONTENT); } -static void -PopupListenerPropertyDtor(void* aObject, nsIAtom* aPropertyName, - void* aPropertyValue, void* aData) -{ - nsIDOMEventListener* listener = - static_cast(aPropertyValue); - if (!listener) { - return; - } - nsEventListenerManager* manager = static_cast(aObject)-> - GetListenerManager(false); - if (manager) { - manager->RemoveEventListenerByType(listener, - NS_LITERAL_STRING("mousedown"), - NS_EVENT_FLAG_BUBBLE | - NS_EVENT_FLAG_SYSTEM_EVENT); - manager->RemoveEventListenerByType(listener, - NS_LITERAL_STRING("contextmenu"), - NS_EVENT_FLAG_BUBBLE | - NS_EVENT_FLAG_SYSTEM_EVENT); - } - NS_RELEASE(listener); -} - nsresult nsXULElement::AddPopupListener(nsIAtom* aName) { // Add a popup listener to the element bool isContext = (aName == nsGkAtoms::context || aName == nsGkAtoms::contextmenu); - nsIAtom* listenerAtom = isContext ? - nsGkAtoms::contextmenulistener : - nsGkAtoms::popuplistener; + uint32_t listenerFlag = isContext ? + XUL_ELEMENT_HAS_CONTENTMENU_LISTENER : + XUL_ELEMENT_HAS_POPUP_LISTENER; - nsCOMPtr popupListener = - static_cast(GetProperty(listenerAtom)); - if (popupListener) { - // Popup listener is already installed. + if (HasFlag(listenerFlag)) { return NS_OK; } - popupListener = new nsXULPopupListener(this, isContext); + nsCOMPtr listener = + new nsXULPopupListener(this, isContext); // Add the popup as a listener on this element. nsEventListenerManager* manager = GetListenerManager(true); - NS_ENSURE_TRUE(manager, NS_ERROR_FAILURE); - nsresult rv = SetProperty(listenerAtom, popupListener, - PopupListenerPropertyDtor, true); - NS_ENSURE_SUCCESS(rv, rv); - // Want the property to have a reference to the listener. - nsIDOMEventListener* listener = nullptr; - popupListener.swap(listener); + SetFlags(listenerFlag); if (isContext) { manager->AddEventListenerByType(listener, diff --git a/content/xul/content/src/nsXULElement.h b/content/xul/content/src/nsXULElement.h index 098e67ba6257..53aa453cc5f0 100644 --- a/content/xul/content/src/nsXULElement.h +++ b/content/xul/content/src/nsXULElement.h @@ -321,11 +321,13 @@ public: // XUL element specific bits enum { - XUL_ELEMENT_TEMPLATE_GENERATED = XUL_ELEMENT_FLAG_BIT(0) + XUL_ELEMENT_TEMPLATE_GENERATED = XUL_ELEMENT_FLAG_BIT(0), + XUL_ELEMENT_HAS_CONTENTMENU_LISTENER = XUL_ELEMENT_FLAG_BIT(1), + XUL_ELEMENT_HAS_POPUP_LISTENER = XUL_ELEMENT_FLAG_BIT(2) }; -// Make sure we have space for our bit -PR_STATIC_ASSERT(ELEMENT_TYPE_SPECIFIC_BITS_OFFSET < 32); +// Make sure we have space for our bits +PR_STATIC_ASSERT((ELEMENT_TYPE_SPECIFIC_BITS_OFFSET + 2) < 32); #undef XUL_ELEMENT_FLAG_BIT diff --git a/content/xul/content/src/nsXULPopupListener.cpp b/content/xul/content/src/nsXULPopupListener.cpp index d4f2f47eaaf4..26577548f2cd 100644 --- a/content/xul/content/src/nsXULPopupListener.cpp +++ b/content/xul/content/src/nsXULPopupListener.cpp @@ -33,7 +33,7 @@ #include "nsHTMLReflowState.h" #include "nsIObjectLoadingContent.h" #include "mozilla/Preferences.h" -#include "mozilla/dom/Element.h" +#include "mozilla/dom/FragmentOrElement.h" // for event firing in context menus #include "nsPresContext.h" @@ -52,7 +52,8 @@ using namespace mozilla; #define NS_CONTEXT_MENU_IS_MOUSEUP 1 #endif -nsXULPopupListener::nsXULPopupListener(nsIDOMElement *aElement, bool aIsContext) +nsXULPopupListener::nsXULPopupListener(mozilla::dom::Element* aElement, + bool aIsContext) : mElement(aElement), mPopupContent(nullptr), mIsContext(aIsContext) { } @@ -66,6 +67,25 @@ NS_IMPL_CYCLE_COLLECTION_2(nsXULPopupListener, mElement, mPopupContent) NS_IMPL_CYCLE_COLLECTING_ADDREF(nsXULPopupListener) NS_IMPL_CYCLE_COLLECTING_RELEASE(nsXULPopupListener) +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_BEGIN(nsXULPopupListener) + // If the owner, mElement, can be skipped, so can we. + if (tmp->mElement) { + return mozilla::dom::FragmentOrElement::CanSkip(tmp->mElement, true); + } +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_END + +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_IN_CC_BEGIN(nsXULPopupListener) + if (tmp->mElement) { + return mozilla::dom::FragmentOrElement::CanSkipInCC(tmp->mElement); + } +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_IN_CC_END + +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_BEGIN(nsXULPopupListener) + if (tmp->mElement) { + return mozilla::dom::FragmentOrElement::CanSkipThis(tmp->mElement); + } +NS_IMPL_CYCLE_COLLECTION_CAN_SKIP_THIS_END + NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(nsXULPopupListener) NS_INTERFACE_MAP_ENTRY(nsIDOMEventListener) NS_INTERFACE_MAP_ENTRY(nsISupports) @@ -307,44 +327,37 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) { nsresult rv = NS_OK; - nsAutoString type(NS_LITERAL_STRING("popup")); - if (mIsContext) - type.AssignLiteral("context"); + nsIAtom* type = mIsContext ? nsGkAtoms::context : nsGkAtoms::popup; nsAutoString identifier; - mElement->GetAttribute(type, identifier); + mElement->GetAttr(kNameSpaceID_None, type, identifier); if (identifier.IsEmpty()) { - if (type.EqualsLiteral("popup")) - mElement->GetAttribute(NS_LITERAL_STRING("menu"), identifier); - else if (type.EqualsLiteral("context")) - mElement->GetAttribute(NS_LITERAL_STRING("contextmenu"), identifier); + if (type == nsGkAtoms::popup) { + mElement->GetAttr(kNameSpaceID_None, nsGkAtoms::menu, identifier); + } else { + mElement->GetAttr(kNameSpaceID_None, nsGkAtoms::contextmenu, identifier); + } if (identifier.IsEmpty()) return rv; } // Try to find the popup content and the document. - nsCOMPtr content = do_QueryInterface(mElement); - nsCOMPtr document = content->GetDocument(); - - // Turn the document into a DOM document so we can use getElementById - nsCOMPtr domDocument = do_QueryInterface(document); - if (!domDocument) { - NS_ERROR("Popup attached to an element that isn't in XUL!"); + nsCOMPtr document = mElement->GetDocument(); + if (!document) { + NS_WARNING("No document!"); return NS_ERROR_FAILURE; } // Handle the _child case for popups and context menus - nsCOMPtr popupElement; - + nsCOMPtr popup; if (identifier.EqualsLiteral("_child")) { - nsCOMPtr popup = GetImmediateChild(content, nsGkAtoms::menupopup); - if (popup) - popupElement = do_QueryInterface(popup); - else { - nsCOMPtr nsDoc(do_QueryInterface(domDocument)); + popup = GetImmediateChild(mElement, nsGkAtoms::menupopup); + if (!popup) { + nsCOMPtr nsDoc(do_QueryInterface(document)); nsCOMPtr list; - nsDoc->GetAnonymousNodes(mElement, getter_AddRefs(list)); + nsCOMPtr el = do_QueryInterface(mElement); + nsDoc->GetAnonymousNodes(el, getter_AddRefs(list)); if (list) { uint32_t ctr,listLength; nsCOMPtr node; @@ -355,15 +368,13 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) if (childContent->NodeInfo()->Equals(nsGkAtoms::menupopup, kNameSpaceID_XUL)) { - popupElement = do_QueryInterface(childContent); + popup.swap(childContent); break; } } } } - } - else if (NS_FAILED(rv = domDocument->GetElementById(identifier, - getter_AddRefs(popupElement)))) { + } else if (!(popup = document->GetElementById(identifier))) { // Use getElementById to obtain the popup content and gracefully fail if // we didn't find any popup content in the document. NS_ERROR("GetElementById had some kind of spasm."); @@ -371,12 +382,11 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) } // return if no popup was found or the popup is the element itself. - if ( !popupElement || popupElement == mElement) + if (!popup || popup == mElement) return NS_OK; // Submenus can't be used as context menus or popups, bug 288763. // Similar code also in nsXULTooltipListener::GetTooltipFor. - nsCOMPtr popup = do_QueryInterface(popupElement); nsIContent* parent = popup->GetParent(); if (parent) { nsMenuFrame* menu = do_QueryFrame(parent->GetPrimaryFrame()); @@ -397,7 +407,7 @@ nsXULPopupListener::LaunchPopup(nsIDOMEvent* aEvent, nsIContent* aTargetContent) (mPopupContent->HasAttr(kNameSpaceID_None, nsGkAtoms::position) || (mPopupContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popupanchor) && mPopupContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popupalign)))) { - pm->ShowPopup(mPopupContent, content, EmptyString(), 0, 0, + pm->ShowPopup(mPopupContent, mElement, EmptyString(), 0, 0, false, true, false, aEvent); } else { diff --git a/content/xul/content/src/nsXULPopupListener.h b/content/xul/content/src/nsXULPopupListener.h index 311e992293ab..d401d9554918 100644 --- a/content/xul/content/src/nsXULPopupListener.h +++ b/content/xul/content/src/nsXULPopupListener.h @@ -12,7 +12,7 @@ #include "nsCOMPtr.h" -#include "nsIContent.h" +#include "mozilla/dom/Element.h" #include "nsIDOMElement.h" #include "nsIDOMMouseEvent.h" #include "nsIDOMEventListener.h" @@ -25,12 +25,12 @@ public: // false, the popup opens on left click on aElement or a descendant. If // aIsContext is true, the popup is a context menu which opens on a // context menu event. - nsXULPopupListener(nsIDOMElement *aElement, bool aIsContext); + nsXULPopupListener(mozilla::dom::Element* aElement, bool aIsContext); virtual ~nsXULPopupListener(void); // nsISupports NS_DECL_CYCLE_COLLECTING_ISUPPORTS - NS_DECL_CYCLE_COLLECTION_CLASS(nsXULPopupListener) + NS_DECL_CYCLE_COLLECTION_SKIPPABLE_CLASS(nsXULPopupListener) NS_DECL_NSIDOMEVENTLISTENER protected: @@ -48,7 +48,7 @@ private: #endif // |mElement| is the node to which this listener is attached. - nsCOMPtr mElement; + nsCOMPtr mElement; // The popup that is getting shown on top of mElement. nsCOMPtr mPopupContent; From 39ef9c4b1c5568f3f08b86510c8a40b7811f8ae2 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Tue, 16 Oct 2012 17:52:10 +0300 Subject: [PATCH 2/9] Bug 797806 - Helper method to handle stringified JSON in C++, r=khuey --- dom/bindings/BindingUtils.cpp | 20 ++++++++++++++++++++ dom/bindings/BindingUtils.h | 9 +++++++++ dom/bindings/Codegen.py | 15 +++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index 2f472a103f19..718e4536c89a 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -11,6 +11,7 @@ #include "AccessCheck.h" #include "WrapperFactory.h" #include "xpcprivate.h" +#include "nsContentUtils.h" #include "XPCQuickStubs.h" #include "nsIXPConnect.h" @@ -739,5 +740,24 @@ SetXrayExpandoChain(JSObject* obj, JSObject* chain) } } +JSContext* +MainThreadDictionaryBase::ParseJSON(const nsAString& aJSON, + mozilla::Maybe& aAr, + mozilla::Maybe& aAc, + JS::Value& aVal) +{ + JSContext* cx = nsContentUtils::ThreadJSContextStack()->GetSafeJSContext(); + NS_ENSURE_TRUE(cx, nullptr); + JSObject* global = JS_GetGlobalObject(cx); + aAr.construct(cx); + aAc.construct(cx, global); + if (!JS_ParseJSON(cx, + static_cast(PromiseFlatString(aJSON).get()), + aJSON.Length(), &aVal)) { + return nullptr; + } + return cx; +} + } // namespace dom } // namespace mozilla diff --git a/dom/bindings/BindingUtils.h b/dom/bindings/BindingUtils.h index 3af15b9df23c..980a67bae6b0 100644 --- a/dom/bindings/BindingUtils.h +++ b/dom/bindings/BindingUtils.h @@ -1227,6 +1227,15 @@ MustInheritFromNonRefcountedDOMObject(NonRefcountedDOMObject*) JSObject* GetXrayExpandoChain(JSObject *obj); void SetXrayExpandoChain(JSObject *obj, JSObject *chain); +struct MainThreadDictionaryBase +{ +protected: + JSContext* ParseJSON(const nsAString& aJSON, + mozilla::Maybe& aAr, + mozilla::Maybe& aAc, + JS::Value& aVal); +}; + } // namespace dom } // namespace mozilla diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 5fcc6e65400e..eb3628793794 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -5390,6 +5390,8 @@ class CGDictionary(CGThing): d = self.dictionary if d.parent: inheritance = ": public %s " % self.makeClassName(d.parent) + elif not self.workers: + inheritance = ": public MainThreadDictionaryBase " else: inheritance = "" memberDecls = [" %s %s;" % @@ -5400,6 +5402,19 @@ class CGDictionary(CGThing): "struct ${selfName} ${inheritance}{\n" " ${selfName}() {}\n" " bool Init(JSContext* cx, const JS::Value& val);\n" + " \n" + + (" bool Init(const nsAString& aJSON)\n" + " {\n" + " if (aJSON.IsEmpty()) {\n" + " return true;\n" + " }\n" + " mozilla::Maybe ar;\n" + " mozilla::Maybe ac;\n" + " jsval json = JSVAL_VOID;\n" + " JSContext* cx = ParseJSON(aJSON, ar, ac, json);\n" + " NS_ENSURE_TRUE(cx, false);\n" + " return Init(cx, json);\n" + " }\n" if not self.workers else "") + "\n" + "\n".join(memberDecls) + "\n" "private:\n" From dd922cba69898d7d0ba41af585a4d5f6cfd1cf4d Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Tue, 16 Oct 2012 18:37:44 +0300 Subject: [PATCH 3/9] Bug 797806 - Helper method to handle stringified JSON in C++, part2, r=khuey --- dom/bindings/BindingUtils.cpp | 3 +++ dom/bindings/Codegen.py | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dom/bindings/BindingUtils.cpp b/dom/bindings/BindingUtils.cpp index 718e4536c89a..46e9d79a6f01 100644 --- a/dom/bindings/BindingUtils.cpp +++ b/dom/bindings/BindingUtils.cpp @@ -751,6 +751,9 @@ MainThreadDictionaryBase::ParseJSON(const nsAString& aJSON, JSObject* global = JS_GetGlobalObject(cx); aAr.construct(cx); aAc.construct(cx, global); + if (aJSON.IsEmpty()) { + return cx; + } if (!JS_ParseJSON(cx, static_cast(PromiseFlatString(aJSON).get()), aJSON.Length(), &aVal)) { diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index eb3628793794..94909b8075a9 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -5405,9 +5405,6 @@ class CGDictionary(CGThing): " \n" + (" bool Init(const nsAString& aJSON)\n" " {\n" - " if (aJSON.IsEmpty()) {\n" - " return true;\n" - " }\n" " mozilla::Maybe ar;\n" " mozilla::Maybe ac;\n" " jsval json = JSVAL_VOID;\n" From 9b0355d9ac2593bc039acbffbfc26d6bb754e5c3 Mon Sep 17 00:00:00 2001 From: Chris AtLee Date: Thu, 20 Sep 2012 12:31:03 -0400 Subject: [PATCH 4/9] Bug 791209: Update l10n mozconfigs for nightly builds. r=ted DONTBUILD --- browser/config/mozconfigs/linux32/l10n-mozconfig | 3 +++ browser/config/mozconfigs/linux64/l10n-mozconfig | 3 +++ .../mozconfigs/macosx-universal/l10n-mozconfig | 13 +++++++++++++ 3 files changed, 19 insertions(+) create mode 100644 browser/config/mozconfigs/macosx-universal/l10n-mozconfig diff --git a/browser/config/mozconfigs/linux32/l10n-mozconfig b/browser/config/mozconfigs/linux32/l10n-mozconfig index 0b216be44508..06ad165ee522 100644 --- a/browser/config/mozconfigs/linux32/l10n-mozconfig +++ b/browser/config/mozconfigs/linux32/l10n-mozconfig @@ -3,6 +3,9 @@ ac_add_options --enable-official-branding ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL} ac_add_options --enable-update-packaging +# Avoid dependency on libstdc++ 4.5 +ac_add_options --enable-stdcxx-compat + . $topsrcdir/build/unix/mozconfig.linux diff --git a/browser/config/mozconfigs/linux64/l10n-mozconfig b/browser/config/mozconfigs/linux64/l10n-mozconfig index 0b216be44508..06ad165ee522 100644 --- a/browser/config/mozconfigs/linux64/l10n-mozconfig +++ b/browser/config/mozconfigs/linux64/l10n-mozconfig @@ -3,6 +3,9 @@ ac_add_options --enable-official-branding ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL} ac_add_options --enable-update-packaging +# Avoid dependency on libstdc++ 4.5 +ac_add_options --enable-stdcxx-compat + . $topsrcdir/build/unix/mozconfig.linux diff --git a/browser/config/mozconfigs/macosx-universal/l10n-mozconfig b/browser/config/mozconfigs/macosx-universal/l10n-mozconfig new file mode 100644 index 000000000000..881d4167b934 --- /dev/null +++ b/browser/config/mozconfigs/macosx-universal/l10n-mozconfig @@ -0,0 +1,13 @@ +. "$topsrcdir/browser/config/mozconfigs/common" + +ac_add_options --with-l10n-base=../../l10n-central + +if [ "${MOZ_UPDATE_CHANNEL}" = "release" -o "${MOZ_UPDATE_CHANNEL}" = "beta" ]; then +ac_add_options --enable-official-branding +fi + +ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL} +ac_add_options --enable-update-packaging +ac_add_options --with-ccache + +. "$topsrcdir/build/mozconfig.common.override" From 3d8aac68f7cf8b90e43e9358d17b0882decb2b4d Mon Sep 17 00:00:00 2001 From: Doug Turner Date: Sat, 13 Oct 2012 08:20:14 -0700 Subject: [PATCH 5/9] Bug 791816 - Use a different error string when we may overwrite a file location. r=sicking --- dom/devicestorage/DeviceStorageRequestParent.cpp | 8 ++++++++ dom/devicestorage/nsDeviceStorage.cpp | 10 ++++++++++ dom/devicestorage/nsDeviceStorage.h | 1 + 3 files changed, 19 insertions(+) diff --git a/dom/devicestorage/DeviceStorageRequestParent.cpp b/dom/devicestorage/DeviceStorageRequestParent.cpp index 4e19c6f1e854..c5252a1d5811 100644 --- a/dom/devicestorage/DeviceStorageRequestParent.cpp +++ b/dom/devicestorage/DeviceStorageRequestParent.cpp @@ -248,6 +248,14 @@ DeviceStorageRequestParent::WriteFileEvent::CancelableRun() return NS_OK; } + bool check = false; + mFile->mFile->Exists(&check); + if (check) { + nsCOMPtr event = new PostErrorEvent(mParent, POST_ERROR_EVENT_FILE_EXISTS); + NS_DispatchToMainThread(event); + return NS_OK; + } + nsresult rv = mFile->Write(mInputStream); if (NS_FAILED(rv)) { diff --git a/dom/devicestorage/nsDeviceStorage.cpp b/dom/devicestorage/nsDeviceStorage.cpp index d15b46b4b15d..e2ca9778d9d8 100644 --- a/dom/devicestorage/nsDeviceStorage.cpp +++ b/dom/devicestorage/nsDeviceStorage.cpp @@ -1285,6 +1285,16 @@ public: nsCOMPtr stream; mBlob->GetInternalStream(getter_AddRefs(stream)); + bool check = false; + mFile->mFile->Exists(&check); + if (check) { + nsCOMPtr event = new PostErrorEvent(mRequest, + POST_ERROR_EVENT_FILE_EXISTS, + mFile); + NS_DispatchToMainThread(event); + return NS_OK; + } + nsresult rv = mFile->Write(stream); if (NS_FAILED(rv)) { diff --git a/dom/devicestorage/nsDeviceStorage.h b/dom/devicestorage/nsDeviceStorage.h index cad3e23e9e67..dfed1661e962 100644 --- a/dom/devicestorage/nsDeviceStorage.h +++ b/dom/devicestorage/nsDeviceStorage.h @@ -32,6 +32,7 @@ class nsPIDOMWindow; #include "DeviceStorageRequestChild.h" +#define POST_ERROR_EVENT_FILE_EXISTS "File already exists" #define POST_ERROR_EVENT_FILE_DOES_NOT_EXIST "File location doesn't exists" #define POST_ERROR_EVENT_FILE_NOT_ENUMERABLE "File location is not enumerable" #define POST_ERROR_EVENT_PERMISSION_DENIED "Permission Denied" From 6c7867e2e2d2a95c757b3baa93f6d6a2c1d39cd7 Mon Sep 17 00:00:00 2001 From: Doug Turner Date: Sat, 13 Oct 2012 08:20:14 -0700 Subject: [PATCH 6/9] Bug 754350 - Clean up Device Storage error strings. r=sicking --- dom/devicestorage/nsDeviceStorage.cpp | 81 +++++-------------- dom/devicestorage/nsDeviceStorage.h | 13 ++- .../test/test_addCorrectType.html | 2 + dom/devicestorage/test/test_basic.html | 11 +-- dom/devicestorage/test/test_dotdot.html | 8 +- dom/devicestorage/test/test_overwrite.html | 1 + 6 files changed, 34 insertions(+), 82 deletions(-) diff --git a/dom/devicestorage/nsDeviceStorage.cpp b/dom/devicestorage/nsDeviceStorage.cpp index e2ca9778d9d8..2e122dcb59e2 100644 --- a/dom/devicestorage/nsDeviceStorage.cpp +++ b/dom/devicestorage/nsDeviceStorage.cpp @@ -851,44 +851,20 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END class PostErrorEvent : public nsRunnable { public: - PostErrorEvent(nsRefPtr& aRequest, const char* aMessage, DeviceStorageFile* aFile) + PostErrorEvent(nsRefPtr& aRequest, const char* aMessage) { mRequest.swap(aRequest); - BuildErrorString(aMessage, aFile); + CopyASCIItoUTF16(aMessage, mError); } - PostErrorEvent(DOMRequest* aRequest, const char* aMessage, DeviceStorageFile* aFile) + PostErrorEvent(DOMRequest* aRequest, const char* aMessage) : mRequest(aRequest) { - BuildErrorString(aMessage, aFile); + CopyASCIItoUTF16(aMessage, mError); } ~PostErrorEvent() {} - void BuildErrorString(const char* aMessage, DeviceStorageFile* aFile) - { - nsAutoString fullPath; - - if (aFile && aFile->mFile) { - aFile->mFile->GetPath(fullPath); - } - else { - fullPath.Assign(NS_LITERAL_STRING("null file")); - } - - mError = NS_ConvertASCIItoUTF16(aMessage); - mError.Append(NS_LITERAL_STRING(" file path = ")); - mError.Append(fullPath.get()); - mError.Append(NS_LITERAL_STRING(" path = ")); - - if (aFile) { - mError.Append(aFile->mPath); - } - else { - mError.Append(NS_LITERAL_STRING("null path")); - } - } - NS_IMETHOD Run() { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); @@ -1006,9 +982,7 @@ public: bool check; mFile->mFile->IsDirectory(&check); if (!check) { - nsCOMPtr event = new PostErrorEvent(mRequest, - POST_ERROR_EVENT_FILE_NOT_ENUMERABLE, - mFile); + nsCOMPtr event = new PostErrorEvent(mRequest, POST_ERROR_EVENT_FILE_NOT_ENUMERABLE); NS_DispatchToMainThread(event); return NS_OK; } @@ -1093,9 +1067,7 @@ nsDOMDeviceStorageCursor::GetElement(nsIDOMElement * *aRequestingElement) NS_IMETHODIMP nsDOMDeviceStorageCursor::Cancel() { - nsCOMPtr event = new PostErrorEvent(this, - POST_ERROR_EVENT_PERMISSION_DENIED, - mFile); + nsCOMPtr event = new PostErrorEvent(this, POST_ERROR_EVENT_PERMISSION_DENIED); NS_DispatchToMainThread(event); return NS_OK; } @@ -1104,9 +1076,7 @@ NS_IMETHODIMP nsDOMDeviceStorageCursor::Allow() { if (!mFile->IsSafePath()) { - nsCOMPtr r = new PostErrorEvent(this, - POST_ERROR_EVENT_ILLEGAL_FILE_NAME, - mFile); + nsCOMPtr r = new PostErrorEvent(this, POST_ERROR_EVENT_PERMISSION_DENIED); NS_DispatchToMainThread(r); return NS_OK; } @@ -1288,9 +1258,7 @@ public: bool check = false; mFile->mFile->Exists(&check); if (check) { - nsCOMPtr event = new PostErrorEvent(mRequest, - POST_ERROR_EVENT_FILE_EXISTS, - mFile); + nsCOMPtr event = new PostErrorEvent(mRequest, POST_ERROR_EVENT_FILE_EXISTS); NS_DispatchToMainThread(event); return NS_OK; } @@ -1300,9 +1268,7 @@ public: if (NS_FAILED(rv)) { mFile->mFile->Remove(false); - nsCOMPtr event = new PostErrorEvent(mRequest, - POST_ERROR_EVENT_UNKNOWN, - mFile); + nsCOMPtr event = new PostErrorEvent(mRequest, POST_ERROR_EVENT_UNKNOWN); NS_DispatchToMainThread(event); return NS_OK; } @@ -1339,7 +1305,7 @@ public: bool check = false; mFile->mFile->Exists(&check); if (!check) { - r = new PostErrorEvent(mRequest, POST_ERROR_EVENT_FILE_DOES_NOT_EXIST, mFile); + r = new PostErrorEvent(mRequest, POST_ERROR_EVENT_FILE_DOES_NOT_EXIST); } } @@ -1376,7 +1342,7 @@ public: bool check = false; mFile->mFile->Exists(&check); if (check) { - r = new PostErrorEvent(mRequest, POST_ERROR_EVENT_FILE_DOES_NOT_EXIST, mFile); + r = new PostErrorEvent(mRequest, POST_ERROR_EVENT_FILE_DOES_NOT_EXIST); } else { r = new PostResultEvent(mRequest, mFile->mPath); @@ -1537,9 +1503,7 @@ public: NS_IMETHOD Cancel() { - nsCOMPtr event = new PostErrorEvent(mRequest, - POST_ERROR_EVENT_PERMISSION_DENIED, - mFile); + nsCOMPtr event = new PostErrorEvent(mRequest, POST_ERROR_EVENT_PERMISSION_DENIED); NS_DispatchToMainThread(event); return NS_OK; } @@ -1847,12 +1811,11 @@ nsDOMDeviceStorage::AddNamed(nsIDOMBlob *aBlob, nsCOMPtr r; nsRefPtr dsf = new DeviceStorageFile(mStorageType, mRootDirectory, aPath); - if (!typeChecker->Check(mStorageType, dsf->mFile) || + if (!dsf->IsSafePath()) { + r = new PostErrorEvent(request, POST_ERROR_EVENT_PERMISSION_DENIED); + } else if (!typeChecker->Check(mStorageType, dsf->mFile) || !typeChecker->Check(mStorageType, aBlob)) { - r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_TYPE, dsf); - } - else if (!dsf->IsSafePath()) { - r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_FILE_NAME, dsf); + r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_TYPE); } else { r = new DeviceStorageRequest(DeviceStorageRequest::DEVICE_STORAGE_REQUEST_WRITE, @@ -1898,10 +1861,7 @@ nsDOMDeviceStorage::GetInternal(const JS::Value & aPath, JSString* jsstr = JS_ValueToString(aCx, aPath); nsDependentJSString path; if (!path.init(aCx, jsstr)) { - nsRefPtr dsf = new DeviceStorageFile(mStorageType, mRootDirectory); - r = new PostErrorEvent(request, - POST_ERROR_EVENT_NON_STRING_TYPE_UNSUPPORTED, - dsf); + r = new PostErrorEvent(request, POST_ERROR_EVENT_UNKNOWN); NS_DispatchToMainThread(r); return NS_OK; } @@ -1909,7 +1869,7 @@ nsDOMDeviceStorage::GetInternal(const JS::Value & aPath, nsRefPtr dsf = new DeviceStorageFile(mStorageType, mRootDirectory, path); dsf->SetEditable(aEditable); if (!dsf->IsSafePath()) { - r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_FILE_NAME, dsf); + r = new PostErrorEvent(request, POST_ERROR_EVENT_PERMISSION_DENIED); } else { r = new DeviceStorageRequest(DeviceStorageRequest::DEVICE_STORAGE_REQUEST_READ, win, mPrincipal, dsf, request); @@ -1934,8 +1894,7 @@ nsDOMDeviceStorage::Delete(const JS::Value & aPath, JSContext* aCx, nsIDOMDOMReq JSString* jsstr = JS_ValueToString(aCx, aPath); nsDependentJSString path; if (!path.init(aCx, jsstr)) { - nsRefPtr dsf = new DeviceStorageFile(mStorageType, mRootDirectory); - r = new PostErrorEvent(request, POST_ERROR_EVENT_NON_STRING_TYPE_UNSUPPORTED, dsf); + r = new PostErrorEvent(request, POST_ERROR_EVENT_UNKNOWN); NS_DispatchToMainThread(r); return NS_OK; } @@ -1943,7 +1902,7 @@ nsDOMDeviceStorage::Delete(const JS::Value & aPath, JSContext* aCx, nsIDOMDOMReq nsRefPtr dsf = new DeviceStorageFile(mStorageType, mRootDirectory, path); if (!dsf->IsSafePath()) { - r = new PostErrorEvent(request, POST_ERROR_EVENT_ILLEGAL_FILE_NAME, dsf); + r = new PostErrorEvent(request, POST_ERROR_EVENT_PERMISSION_DENIED); } else { r = new DeviceStorageRequest(DeviceStorageRequest::DEVICE_STORAGE_REQUEST_DELETE, diff --git a/dom/devicestorage/nsDeviceStorage.h b/dom/devicestorage/nsDeviceStorage.h index dfed1661e962..279c2977b0db 100644 --- a/dom/devicestorage/nsDeviceStorage.h +++ b/dom/devicestorage/nsDeviceStorage.h @@ -32,15 +32,12 @@ class nsPIDOMWindow; #include "DeviceStorageRequestChild.h" -#define POST_ERROR_EVENT_FILE_EXISTS "File already exists" -#define POST_ERROR_EVENT_FILE_DOES_NOT_EXIST "File location doesn't exists" -#define POST_ERROR_EVENT_FILE_NOT_ENUMERABLE "File location is not enumerable" -#define POST_ERROR_EVENT_PERMISSION_DENIED "Permission Denied" -#define POST_ERROR_EVENT_ILLEGAL_FILE_NAME "Illegal file name" -#define POST_ERROR_EVENT_ILLEGAL_TYPE "Illegal content type" +#define POST_ERROR_EVENT_FILE_EXISTS "NoModificationAllowedError" +#define POST_ERROR_EVENT_FILE_DOES_NOT_EXIST "NotFoundError" +#define POST_ERROR_EVENT_FILE_NOT_ENUMERABLE "TypeMismatchError" +#define POST_ERROR_EVENT_PERMISSION_DENIED "SecurityError" +#define POST_ERROR_EVENT_ILLEGAL_TYPE "TypeMismatchError" #define POST_ERROR_EVENT_UNKNOWN "Unknown" -#define POST_ERROR_EVENT_NON_STRING_TYPE_UNSUPPORTED "Non-string type unsupported" -#define POST_ERROR_EVENT_NOT_IMPLEMENTED "Not implemented" using namespace mozilla; using namespace mozilla::dom; diff --git a/dom/devicestorage/test/test_addCorrectType.html b/dom/devicestorage/test/test_addCorrectType.html index 2920840f91ea..1a4cd8b3f483 100644 --- a/dom/devicestorage/test/test_addCorrectType.html +++ b/dom/devicestorage/test/test_addCorrectType.html @@ -43,6 +43,8 @@ var tests = [ function fail(e) { ok(false, "addSuccess was called"); + ok(e.target.error.name == "TypeMismatchError", "Error must be TypeMismatchError"); + devicestorage_cleanup(); } diff --git a/dom/devicestorage/test/test_basic.html b/dom/devicestorage/test/test_basic.html index 23ce6c142407..28e453740bea 100644 --- a/dom/devicestorage/test/test_basic.html +++ b/dom/devicestorage/test/test_basic.html @@ -13,7 +13,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=717103 - + Mozilla Bug 717103