From 91cace0cc3959cc70d72d05d9807a99ecad1b17f Mon Sep 17 00:00:00 2001 From: Gregory Pappas Date: Wed, 28 Feb 2024 21:29:37 +0000 Subject: [PATCH] Bug 1878401 - part 1 - Pass BrowsingContext to nsIFilePicker::Init instead of mozIDOMWindow r=geckoview-reviewers,win-reviewers,emilio,nika,m_kato,rkraesig This will improve the situation in bug 1878336 Differential Revision: https://phabricator.services.mozilla.com/D200546 --- dom/html/HTMLInputElement.cpp | 16 ++++----- dom/ipc/BrowserParent.cpp | 16 +++++++-- dom/ipc/BrowserParent.h | 3 +- dom/ipc/FilePickerParent.cpp | 20 ++++------- dom/ipc/FilePickerParent.h | 10 ++++-- dom/ipc/PBrowser.ipdl | 2 +- .../geckoview/FilePickerDelegate.sys.mjs | 4 +-- .../content/MockFilePicker.sys.mjs | 6 ++-- widget/nsBaseFilePicker.cpp | 35 +++++++++++++------ widget/nsBaseFilePicker.h | 8 ++--- widget/nsFilePickerProxy.cpp | 19 +++++----- widget/nsFilePickerProxy.h | 5 ++- widget/nsIFilePicker.idl | 18 ++++------ widget/windows/nsFilePicker.cpp | 13 +++---- widget/windows/nsFilePicker.h | 6 ++-- 15 files changed, 93 insertions(+), 88 deletions(-) diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index e52ceec22475..9978eb1ff203 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -779,8 +779,8 @@ nsresult HTMLInputElement::InitFilePicker(FilePickerType aType) { // Get parent nsPIDOMWindow object. nsCOMPtr doc = OwnerDoc(); - nsCOMPtr win = doc->GetWindow(); - if (!win) { + RefPtr bc = doc->GetBrowsingContext(); + if (!bc) { return NS_ERROR_FAILURE; } @@ -793,15 +793,14 @@ nsresult HTMLInputElement::InitFilePicker(FilePickerType aType) { nsAutoString okButtonLabel; if (aType == FILE_PICKER_DIRECTORY) { nsContentUtils::GetMaybeLocalizedString(nsContentUtils::eFORMS_PROPERTIES, - "DirectoryUpload", OwnerDoc(), - title); + "DirectoryUpload", doc, title); nsContentUtils::GetMaybeLocalizedString(nsContentUtils::eFORMS_PROPERTIES, - "DirectoryPickerOkButtonLabel", - OwnerDoc(), okButtonLabel); + "DirectoryPickerOkButtonLabel", doc, + okButtonLabel); } else { nsContentUtils::GetMaybeLocalizedString(nsContentUtils::eFORMS_PROPERTIES, - "FileUpload", OwnerDoc(), title); + "FileUpload", doc, title); } nsCOMPtr filePicker = @@ -818,8 +817,7 @@ nsresult HTMLInputElement::InitFilePicker(FilePickerType aType) { mode = nsIFilePicker::modeOpen; } - nsresult rv = - filePicker->Init(win, title, mode, OwnerDoc()->GetBrowsingContext()); + nsresult rv = filePicker->Init(bc, title, mode); NS_ENSURE_SUCCESS(rv, rv); if (!okButtonLabel.IsEmpty()) { diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index d369556588eb..5d13d2e81487 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -1322,8 +1322,20 @@ mozilla::ipc::IPCResult BrowserParent::RecvPDocAccessibleConstructor( #endif already_AddRefed BrowserParent::AllocPFilePickerParent( - const nsString& aTitle, const nsIFilePicker::Mode& aMode) { - return MakeAndAddRef(aTitle, aMode); + const nsString& aTitle, const nsIFilePicker::Mode& aMode, + const MaybeDiscarded& aBrowsingContext) { + RefPtr browsingContext = + [&]() -> CanonicalBrowsingContext* { + if (aBrowsingContext.IsNullOrDiscarded()) { + return nullptr; + } + if (!aBrowsingContext.get_canonical()->IsOwnedByProcess( + Manager()->ChildID())) { + return nullptr; + } + return aBrowsingContext.get_canonical(); + }(); + return MakeAndAddRef(aTitle, aMode, browsingContext); } already_AddRefed diff --git a/dom/ipc/BrowserParent.h b/dom/ipc/BrowserParent.h index 4d4ae287b476..63a52c6ac466 100644 --- a/dom/ipc/BrowserParent.h +++ b/dom/ipc/BrowserParent.h @@ -595,7 +595,8 @@ class BrowserParent final : public PBrowserParent, const Maybe& aDoubleTapToZoomMetrics); already_AddRefed AllocPFilePickerParent( - const nsString& aTitle, const nsIFilePicker::Mode& aMode); + const nsString& aTitle, const nsIFilePicker::Mode& aMode, + const MaybeDiscarded& aBrowsingContext); bool GetGlobalJSObject(JSContext* cx, JSObject** globalp); diff --git a/dom/ipc/FilePickerParent.cpp b/dom/ipc/FilePickerParent.cpp index ebd24cb0d3f4..ea65f79c236c 100644 --- a/dom/ipc/FilePickerParent.cpp +++ b/dom/ipc/FilePickerParent.cpp @@ -219,25 +219,17 @@ void FilePickerParent::Done(nsIFilePicker::ResultCode aResult) { } bool FilePickerParent::CreateFilePicker() { + if (!mBrowsingContext) { + return false; + } + mFilePicker = do_CreateInstance("@mozilla.org/filepicker;1"); + if (!mFilePicker) { return false; } - auto* browserParent = BrowserParent::GetFrom(Manager()); - auto* browsingContext = browserParent->GetBrowsingContext(); - Element* element = browserParent->GetOwnerElement(); - if (!element) { - return false; - } - - nsCOMPtr window = element->OwnerDoc()->GetWindow(); - if (!window) { - return false; - } - - return NS_SUCCEEDED( - mFilePicker->Init(window, mTitle, mMode, browsingContext)); + return NS_SUCCEEDED(mFilePicker->Init(mBrowsingContext, mTitle, mMode)); } mozilla::ipc::IPCResult FilePickerParent::RecvOpen( diff --git a/dom/ipc/FilePickerParent.h b/dom/ipc/FilePickerParent.h index f0fe0dc2d010..3cdb5ab912bf 100644 --- a/dom/ipc/FilePickerParent.h +++ b/dom/ipc/FilePickerParent.h @@ -12,6 +12,7 @@ #include "nsCOMArray.h" #include "nsThreadUtils.h" #include "mozilla/dom/File.h" +#include "mozilla/dom/BrowsingContext.h" #include "mozilla/dom/PFilePickerParent.h" class nsIFile; @@ -20,8 +21,12 @@ namespace mozilla::dom { class FilePickerParent : public PFilePickerParent { public: - FilePickerParent(const nsString& aTitle, const nsIFilePicker::Mode& aMode) - : mTitle(aTitle), mMode(aMode), mResult(nsIFilePicker::returnOK) {} + FilePickerParent(const nsString& aTitle, const nsIFilePicker::Mode& aMode, + BrowsingContext* aBrowsingContext) + : mTitle(aTitle), + mMode(aMode), + mBrowsingContext(aBrowsingContext), + mResult(nsIFilePicker::returnOK) {} private: virtual ~FilePickerParent(); @@ -93,6 +98,7 @@ class FilePickerParent : public PFilePickerParent { nsString mTitle; nsIFilePicker::Mode mMode; + RefPtr mBrowsingContext; nsIFilePicker::ResultCode mResult; }; diff --git a/dom/ipc/PBrowser.ipdl b/dom/ipc/PBrowser.ipdl index bc126724f467..caef472ec2c6 100644 --- a/dom/ipc/PBrowser.ipdl +++ b/dom/ipc/PBrowser.ipdl @@ -428,7 +428,7 @@ parent: */ async PColorPicker(nsString title, nsString initialColor, nsString[] defaultColors); - async PFilePicker(nsString aTitle, Mode aMode); + async PFilePicker(nsString aTitle, Mode aMode, MaybeDiscardedBrowsingContext aBrowsingContext); /** * Tells the containing widget whether the given input block results in a diff --git a/mobile/android/components/geckoview/FilePickerDelegate.sys.mjs b/mobile/android/components/geckoview/FilePickerDelegate.sys.mjs index 3972017b4051..52d1b9323399 100644 --- a/mobile/android/components/geckoview/FilePickerDelegate.sys.mjs +++ b/mobile/android/components/geckoview/FilePickerDelegate.sys.mjs @@ -15,14 +15,14 @@ const { debug, warn } = GeckoViewUtils.initLogging("FilePickerDelegate"); export class FilePickerDelegate { /* ---------- nsIFilePicker ---------- */ - init(aParent, aTitle, aMode) { + init(aBrowsingContext, aTitle, aMode) { if ( aMode === Ci.nsIFilePicker.modeGetFolder || aMode === Ci.nsIFilePicker.modeSave ) { throw Components.Exception("", Cr.NS_ERROR_NOT_IMPLEMENTED); } - this._prompt = new lazy.GeckoViewPrompter(aParent); + this._prompt = new lazy.GeckoViewPrompter(aBrowsingContext); this._msg = { type: "file", title: aTitle, diff --git a/testing/specialpowers/content/MockFilePicker.sys.mjs b/testing/specialpowers/content/MockFilePicker.sys.mjs index ee2f11af7533..0ce8c25d4e90 100644 --- a/testing/specialpowers/content/MockFilePicker.sys.mjs +++ b/testing/specialpowers/content/MockFilePicker.sys.mjs @@ -48,11 +48,11 @@ export var MockFilePicker = { window: null, pendingPromises: [], - init(window) { - this.window = window; + init(browsingContext) { + this.window = browsingContext.window; this.reset(); - this.factory = newFactory(window); + this.factory = newFactory(this.window); if (!registrar.isCIDRegistered(newClassID)) { oldClassID = registrar.contractIDToCID(CONTRACT_ID); registrar.registerFactory(newClassID, "", CONTRACT_ID, this.factory); diff --git a/widget/nsBaseFilePicker.cpp b/widget/nsBaseFilePicker.cpp index 53be3d80b782..faa78f06f251 100644 --- a/widget/nsBaseFilePicker.cpp +++ b/widget/nsBaseFilePicker.cpp @@ -18,6 +18,7 @@ #include "mozilla/dom/File.h" #include "mozilla/dom/Promise.h" #include "mozilla/dom/Element.h" +#include "mozilla/dom/CanonicalBrowsingContext.h" #include "mozilla/Components.h" #include "mozilla/StaticPrefs_widget.h" #include "WidgetUtils.h" @@ -156,17 +157,16 @@ nsBaseFilePicker::nsBaseFilePicker() nsBaseFilePicker::~nsBaseFilePicker() = default; -NS_IMETHODIMP nsBaseFilePicker::Init( - mozIDOMWindowProxy* aParent, const nsAString& aTitle, - nsIFilePicker::Mode aMode, - mozilla::dom::BrowsingContext* aBrowsingContext) { - MOZ_ASSERT(aParent, - "Null parent passed to filepicker, no file " +NS_IMETHODIMP nsBaseFilePicker::Init(BrowsingContext* aBrowsingContext, + const nsAString& aTitle, + nsIFilePicker::Mode aMode) { + MOZ_ASSERT(XRE_IsParentProcess()); + MOZ_ASSERT(aBrowsingContext, + "Null bc passed to filepicker, no file " "picker for you!"); - mParent = nsPIDOMWindowOuter::From(aParent); - - nsCOMPtr widget = WidgetUtils::DOMWindowToWidget(mParent); + nsCOMPtr widget = + aBrowsingContext->Canonical()->GetParentProcessWidgetContaining(); NS_ENSURE_TRUE(widget, NS_ERROR_FAILURE); mBrowsingContext = aBrowsingContext; @@ -462,6 +462,8 @@ nsBaseFilePicker::GetOkButtonLabel(nsAString& aLabel) { NS_IMETHODIMP nsBaseFilePicker::GetDomFileOrDirectory(nsISupports** aValue) { + MOZ_ASSERT(XRE_IsParentProcess()); + NS_ENSURE_ARG_POINTER(mBrowsingContext); nsCOMPtr localFile; nsresult rv = GetFile(getter_AddRefs(localFile)); NS_ENSURE_SUCCESS(rv, rv); @@ -471,7 +473,10 @@ nsBaseFilePicker::GetDomFileOrDirectory(nsISupports** aValue) { return NS_OK; } - auto* innerParent = mParent ? mParent->GetCurrentInnerWindow() : nullptr; + auto* innerParent = + mBrowsingContext->GetDOMWindow() + ? mBrowsingContext->GetDOMWindow()->GetCurrentInnerWindow() + : nullptr; if (!innerParent) { return NS_ERROR_FAILURE; @@ -485,11 +490,19 @@ NS_IMETHODIMP nsBaseFilePicker::GetDomFileOrDirectoryEnumerator( nsISimpleEnumerator** aValue) { nsCOMPtr iter; + MOZ_ASSERT(XRE_IsParentProcess()); + NS_ENSURE_ARG_POINTER(mBrowsingContext); nsresult rv = GetFiles(getter_AddRefs(iter)); NS_ENSURE_SUCCESS(rv, rv); + auto* parent = mBrowsingContext->GetDOMWindow(); + + if (!parent) { + return NS_ERROR_FAILURE; + } + RefPtr retIter = - new nsBaseFilePickerEnumerator(mParent, iter, mMode); + new nsBaseFilePickerEnumerator(parent, iter, mMode); retIter.forget(aValue); return NS_OK; diff --git a/widget/nsBaseFilePicker.h b/widget/nsBaseFilePicker.h index 3d26a1822e6e..52b8bc779833 100644 --- a/widget/nsBaseFilePicker.h +++ b/widget/nsBaseFilePicker.h @@ -28,9 +28,8 @@ class nsBaseFilePicker : public nsIFilePicker { nsBaseFilePicker(); virtual ~nsBaseFilePicker(); - NS_IMETHOD Init(mozIDOMWindowProxy* aParent, const nsAString& aTitle, - nsIFilePicker::Mode aMode, - mozilla::dom::BrowsingContext* aBrowsingContext) override; + NS_IMETHOD Init(mozilla::dom::BrowsingContext* aBrowsingContext, + const nsAString& aTitle, nsIFilePicker::Mode aMode) override; NS_IMETHOD IsModeSupported(nsIFilePicker::Mode aMode, JSContext* aCx, mozilla::dom::Promise** aPromise) override; #ifndef XP_WIN @@ -70,9 +69,6 @@ class nsBaseFilePicker : public nsIFilePicker { nsCOMPtr mDisplayDirectory; nsString mDisplaySpecialDirectory; - nsCOMPtr mParent; - // The BrowsingContext from which the file picker is being opened. - // Used for content analysis. RefPtr mBrowsingContext; nsIFilePicker::Mode mMode; nsString mOkButtonLabel; diff --git a/widget/nsFilePickerProxy.cpp b/widget/nsFilePickerProxy.cpp index 8777f338cbf4..b71ad0920e15 100644 --- a/widget/nsFilePickerProxy.cpp +++ b/widget/nsFilePickerProxy.cpp @@ -25,19 +25,19 @@ nsFilePickerProxy::nsFilePickerProxy() nsFilePickerProxy::~nsFilePickerProxy() = default; NS_IMETHODIMP -nsFilePickerProxy::Init(mozIDOMWindowProxy* aParent, const nsAString& aTitle, - nsIFilePicker::Mode aMode, - BrowsingContext* aBrowsingContext) { - BrowserChild* browserChild = BrowserChild::GetFrom(aParent); +nsFilePickerProxy::Init(BrowsingContext* aBrowsingContext, + const nsAString& aTitle, nsIFilePicker::Mode aMode) { + BrowserChild* browserChild = + BrowserChild::GetFrom(aBrowsingContext->GetDocShell()); if (!browserChild) { return NS_ERROR_FAILURE; } - mParent = nsPIDOMWindowOuter::From(aParent); - + mBrowsingContext = aBrowsingContext; mMode = aMode; - browserChild->SendPFilePickerConstructor(this, aTitle, aMode); + browserChild->SendPFilePickerConstructor(this, aTitle, aMode, + aBrowsingContext); mIPCActive = true; return NS_OK; @@ -155,8 +155,9 @@ nsFilePickerProxy::Close() { mozilla::ipc::IPCResult nsFilePickerProxy::Recv__delete__( const MaybeInputData& aData, const nsIFilePicker::ResultCode& aResult) { - nsPIDOMWindowInner* inner = - mParent ? mParent->GetCurrentInnerWindow() : nullptr; + auto* inner = mBrowsingContext->GetDOMWindow() + ? mBrowsingContext->GetDOMWindow()->GetCurrentInnerWindow() + : nullptr; if (NS_WARN_IF(!inner)) { return IPC_OK(); diff --git a/widget/nsFilePickerProxy.h b/widget/nsFilePickerProxy.h index 1b6aef13edfb..9f12ded3abb3 100644 --- a/widget/nsFilePickerProxy.h +++ b/widget/nsFilePickerProxy.h @@ -34,9 +34,8 @@ class nsFilePickerProxy : public nsBaseFilePicker, NS_DECL_ISUPPORTS // nsIFilePicker (less what's in nsBaseFilePicker) - NS_IMETHOD Init(mozIDOMWindowProxy* aParent, const nsAString& aTitle, - nsIFilePicker::Mode aMode, - mozilla::dom::BrowsingContext* aBrowsingContext) override; + NS_IMETHOD Init(mozilla::dom::BrowsingContext* aBrowsingContext, + const nsAString& aTitle, nsIFilePicker::Mode aMode) override; NS_IMETHOD AppendFilter(const nsAString& aTitle, const nsAString& aFilter) override; NS_IMETHOD GetCapture(nsIFilePicker::CaptureTarget* aCapture) override; diff --git a/widget/nsIFilePicker.idl b/widget/nsIFilePicker.idl index e9bdedfd4271..db5ac9f7bdf5 100644 --- a/widget/nsIFilePicker.idl +++ b/widget/nsIFilePicker.idl @@ -66,19 +66,13 @@ interface nsIFilePicker : nsISupports * Initialize the file picker widget. The file picker is not valid until this * method is called. * - * @param parent mozIDOMWindow parent. This dialog will be dependent - * on this parent. parent must be non-null. - * @param title The title for the file widget - * @param mode load, save, or get folder - * @param browsingContext [optional] - * The context in which the file picker is being shown. This is - * used for content analysis and can be omitted if chrome is - * showing the file picker. + * @param browsingContext The context in which the file picker is being + * shown, must be non-null. + * @param title The title for the file widget + * @param mode load, save, or get folder + * */ - void init(in mozIDOMWindowProxy parent, - in AString title, - in nsIFilePicker_Mode mode, - [optional] in BrowsingContext browsingContext); + void init(in BrowsingContext browsingContext, in AString title, in nsIFilePicker_Mode mode); /** * Returns a Promise that resolves to true if the passed nsIFilePicker mode diff --git a/widget/windows/nsFilePicker.cpp b/widget/windows/nsFilePicker.cpp index 310c54bb40a0..e9365e675531 100644 --- a/widget/windows/nsFilePicker.cpp +++ b/widget/windows/nsFilePicker.cpp @@ -94,19 +94,14 @@ nsFilePicker::nsFilePicker() : mSelectedType(1) {} NS_IMPL_ISUPPORTS(nsFilePicker, nsIFilePicker) NS_IMETHODIMP nsFilePicker::Init( - mozIDOMWindowProxy* aParent, const nsAString& aTitle, - nsIFilePicker::Mode aMode, - mozilla::dom::BrowsingContext* aBrowsingContext) { + mozilla::dom::BrowsingContext* aBrowsingContext, const nsAString& aTitle, + nsIFilePicker::Mode aMode) { // Don't attempt to open a real file-picker in headless mode. if (gfxPlatform::IsHeadless()) { return nsresult::NS_ERROR_NOT_AVAILABLE; } - nsCOMPtr window = do_QueryInterface(aParent); - nsIDocShell* docShell = window ? window->GetDocShell() : nullptr; - mLoadContext = do_QueryInterface(docShell); - - return nsBaseFilePicker::Init(aParent, aTitle, aMode, aBrowsingContext); + return nsBaseFilePicker::Init(aBrowsingContext, aTitle, aMode); } namespace mozilla::detail { @@ -1053,7 +1048,7 @@ void nsFilePicker::RememberLastUsedDirectory() { } bool nsFilePicker::IsPrivacyModeEnabled() { - return mLoadContext && mLoadContext->UsePrivateBrowsing(); + return mBrowsingContext && mBrowsingContext->UsePrivateBrowsing(); } bool nsFilePicker::IsDefaultPathLink() { diff --git a/widget/windows/nsFilePicker.h b/widget/windows/nsFilePicker.h index 1938b8bcb619..59108bc0dd32 100644 --- a/widget/windows/nsFilePicker.h +++ b/widget/windows/nsFilePicker.h @@ -66,9 +66,8 @@ class nsFilePicker final : public nsBaseWinFilePicker { public: nsFilePicker(); - NS_IMETHOD Init(mozIDOMWindowProxy* aParent, const nsAString& aTitle, - nsIFilePicker::Mode aMode, - mozilla::dom::BrowsingContext* aBrowsingContext) override; + NS_IMETHOD Init(mozilla::dom::BrowsingContext* aBrowsingContext, + const nsAString& aTitle, nsIFilePicker::Mode aMode) override; NS_DECL_ISUPPORTS @@ -117,7 +116,6 @@ class nsFilePicker final : public nsBaseWinFilePicker { bool IsDefaultPathLink(); bool IsDefaultPathHtml(); - nsCOMPtr mLoadContext; nsCOMPtr mParentWidget; nsString mTitle; nsCString mFile;