corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1561015: Part 2 - Return BrowsingContext from openWindow2. r=bzbarsky

Browse Source 
There are some unfortunate aspects of openWindow() and openWindow2() that make
this difficult. Namely, they sometimes return top-level chrome windows, and
sometimes a single content window from the top-level chrome window that they
open, depending on how they're called.

There really isn't any reason to return a BrowsingContext rather than a chrome
window in the former case, but there also really isn't a way to make the API
polymorphic in a way that would allow handling the two cases differently. So
at some point, the two cases should ideally be split into separate APIs rather
than separate special cases of a single API.

In the mean time, I've left openWindow() returning local mozIDOMWindowProxy
objects, since it isn't used by the DOM window.open() or openDialog() APIs,
and only updated openWindow2(). As a follow-up, we should remove both
openWindow() and openWindow2(), and replace them with openChromeWindow() and
openContentWindow() (or similar) methods which make it immediately clear what
type of window is being returned.

Differential Revision: https://phabricator.services.mozilla.com/D35689
This commit is contained in:
Kris Maglione
2019-08-02 20:48:40 +00:00
parent 1bb60064a6
commit 7575d4eb1d
6 changed files with 41 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
dom/base/nsGlobalWindowOuter.cpp
View File

@@ -7275,7 +7275,6 @@ nsresult nsGlobalWindowOuter::OpenInternal(
// dialog is open.
AutoPopupStatePusher popupStatePusher(PopupBlocker::openAbused, true);
nsCOMPtr<mozIDOMWindowProxy> win;
if (!aCalledNoScript) {
// We asserted at the top of this function that aNavigate is true for
// !aCalledNoScript.
@@ -7283,7 +7282,7 @@ nsresult nsGlobalWindowOuter::OpenInternal(
this, url.IsVoid() ? nullptr : url.get(), name_ptr, options_ptr,
/* aCalledFromScript = */ true, aDialog, aNavigate, argv,
isPopupSpamWindow, forceNoOpener, forceNoReferrer, aLoadState,
getter_AddRefs(win));
getter_AddRefs(domReturn));
} else {
// Force a system caller here so that the window watcher won't screw us
// up. We do NOT want this case looking at the JS context on the stack
@@ -7303,10 +7302,7 @@ nsresult nsGlobalWindowOuter::OpenInternal(
this, url.IsVoid() ? nullptr : url.get(), name_ptr, options_ptr,
/* aCalledFromScript = */ false, aDialog, aNavigate, aExtraArgument,
isPopupSpamWindow, forceNoOpener, forceNoReferrer, aLoadState,
getter_AddRefs(win));
}
if (win) {
domReturn = nsPIDOMWindowOuter::From(win)->GetBrowsingContext();
getter_AddRefs(domReturn));
}
}

12
dom/clients/manager/ClientOpenWindowUtils.cpp
View File

@@ -9,6 +9,7 @@
#include "ClientInfo.h"
#include "ClientState.h"
#include "mozilla/SystemGroup.h"
#include "mozilla/ResultExtensions.h"
#include "nsContentUtils.h"
#include "nsFocusManager.h"
#include "nsIBrowserDOMWindow.h"
@@ -211,8 +212,7 @@ nsresult OpenWindow(const ClientOpenWindowArgs& aArgs, BrowsingContext** aBC) {
return rv;
}
nsCOMPtr<mozIDOMWindowProxy> newWindow;
rv = pwwatch->OpenWindow2(
MOZ_TRY(pwwatch->OpenWindow2(
nullptr, spec.get(), nullptr, nullptr, false, false, true, nullptr,
// Not a spammy popup; we got permission, we swear!
/* aIsPopupSpam = */ false,
@@ -221,13 +221,7 @@ nsresult OpenWindow(const ClientOpenWindowArgs& aArgs, BrowsingContext** aBC) {
// window.
/* aForceNoOpener = */ false,
/* aForceNoReferrer = */ false,
/* aLoadInfp = */ nullptr, getter_AddRefs(newWindow));
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
RefPtr<BrowsingContext> bc(
nsPIDOMWindowOuter::From(newWindow)->GetBrowsingContext());
bc.forget(aBC);
/* aLoadInfp = */ nullptr, aBC));
MOZ_DIAGNOSTIC_ASSERT(*aBC);
return NS_OK;
}

5
dom/ipc/ContentChild.cpp
View File

@@ -1104,10 +1104,7 @@ nsresult ContentChild::ProvideWindowCommon(
newChild->RecvLoadURL(urlToLoad, showInfo);
}
nsCOMPtr<nsPIDOMWindowOuter> win =
do_GetInterface(newChild->WebNavigation());
RefPtr<BrowsingContext> bc(win->GetBrowsingContext());
bc.forget(aReturn);
browsingContext.forget(aReturn);
};
// NOTE: Capturing by reference here is safe, as this function won't return

21
toolkit/components/windowwatcher/nsPIWindowWatcher.idl
View File

@@ -14,6 +14,7 @@
class nsDocShellLoadState;
%}
webidl BrowsingContext;
interface mozIDOMWindowProxy;
interface nsIDOMWindow;
interface nsISimpleEnumerator;
@@ -81,16 +82,16 @@ interface nsPIWindowWatcher : nsISupports
(which is determined based on the JS stack and the value of
aParent). This is not guaranteed, however.
*/
mozIDOMWindowProxy openWindow2(in mozIDOMWindowProxy aParent, in string aUrl,
in string aName, in string aFeatures,
in boolean aCalledFromScript,
in boolean aDialog,
in boolean aNavigate,
in nsISupports aArgs,
in boolean aIsPopupSpam,
in boolean aForceNoOpener,
in boolean aForceNoReferrer,
in nsDocShellLoadStatePtr aLoadState);
BrowsingContext openWindow2(in mozIDOMWindowProxy aParent, in string aUrl,
in string aName, in string aFeatures,
in boolean aCalledFromScript,
in boolean aDialog,
in boolean aNavigate,
in nsISupports aArgs,
in boolean aIsPopupSpam,
in boolean aForceNoOpener,
in boolean aForceNoReferrer,
in nsDocShellLoadStatePtr aLoadState);
/**
* Opens a new window so that the window that aOpeningTab belongs to

36
toolkit/components/windowwatcher/nsWindowWatcher.cpp
View File

@@ -61,6 +61,7 @@
#include "mozilla/CheckedInt.h"
#include "mozilla/NullPrincipal.h"
#include "mozilla/Preferences.h"
#include "mozilla/ResultExtensions.h"
#include "mozilla/dom/Element.h"
#include "mozilla/dom/Storage.h"
#include "mozilla/dom/ScriptSettings.h"
@@ -287,13 +288,17 @@ nsWindowWatcher::OpenWindow(mozIDOMWindowProxy* aParent, const char* aUrl,
}
bool dialog = (argc != 0);
return OpenWindowInternal(aParent, aUrl, aName, aFeatures,
/* calledFromJS = */ false, dialog,
/* navigate = */ true, argv,
/* aIsPopupSpam = */ false,
/* aForceNoOpener = */ false,
/* aForceNoReferrer = */ false,
/* aLoadState = */ nullptr, aResult);
RefPtr<BrowsingContext> bc;
MOZ_TRY(OpenWindowInternal(aParent, aUrl, aName, aFeatures,
/* calledFromJS = */ false, dialog,
/* navigate = */ true, argv,
/* aIsPopupSpam = */ false,
/* aForceNoOpener = */ false,
/* aForceNoReferrer = */ false,
/* aLoadState = */ nullptr, getter_AddRefs(bc)));
nsCOMPtr<mozIDOMWindowProxy> win(bc->GetDOMWindow());
win.forget(aResult);
return NS_OK;
}
struct SizeSpec {
@@ -350,7 +355,7 @@ nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy* aParent, const char* aUrl,
bool aIsPopupSpam, bool aForceNoOpener,
bool aForceNoReferrer,
nsDocShellLoadState* aLoadState,
mozIDOMWindowProxy** aResult) {
BrowsingContext** aResult) {
nsCOMPtr<nsIArray> argv = ConvertArgsToArray(aArguments);
uint32_t argc = 0;
@@ -577,7 +582,7 @@ nsresult nsWindowWatcher::OpenWindowInternal(
const char* aFeatures, bool aCalledFromJS, bool aDialog, bool aNavigate,
nsIArray* aArgv, bool aIsPopupSpam, bool aForceNoOpener,
bool aForceNoReferrer, nsDocShellLoadState* aLoadState,
mozIDOMWindowProxy** aResult) {
BrowsingContext** aResult) {
MOZ_ASSERT_IF(aForceNoReferrer, aForceNoOpener);
nsresult rv = NS_OK;
@@ -926,12 +931,17 @@ nsresult nsWindowWatcher::OpenWindowInternal(
newDocShell->SetSandboxFlags(activeDocsSandboxFlags);
}
nsCOMPtr<mozIDOMWindowProxy> win;
rv = ReadyOpenedDocShellItem(newDocShellItem, parentWindow, windowIsNew,
aForceNoOpener, aResult);
aForceNoOpener, getter_AddRefs(win));
if (NS_FAILED(rv)) {
return rv;
}
RefPtr<BrowsingContext> bc(
nsPIDOMWindowOuter::From(win)->GetBrowsingContext());
bc.forget(aResult);
if (isNewToplevelWindow) {
nsCOMPtr<nsIDocShellTreeOwner> newTreeOwner;
newDocShellItem->GetTreeOwner(getter_AddRefs(newTreeOwner));
@@ -940,7 +950,7 @@ nsresult nsWindowWatcher::OpenWindowInternal(
if ((aDialog || windowIsModalContentDialog) && aArgv) {
// Set the args on the new window.
nsCOMPtr<nsPIDOMWindowOuter> piwin(do_QueryInterface(*aResult));
nsCOMPtr<nsPIDOMWindowOuter> piwin(do_QueryInterface(win));
NS_ENSURE_TRUE(piwin, NS_ERROR_UNEXPECTED);
rv = piwin->SetArguments(aArgv);
@@ -1009,7 +1019,7 @@ nsresult nsWindowWatcher::OpenWindowInternal(
// the JS stack, just use the principal of our parent window. In those
// cases we do _not_ set the parent window principal as the owner of the
// load--since we really don't know who the owner is, just leave it null.
nsCOMPtr<nsPIDOMWindowOuter> newWindow = do_QueryInterface(*aResult);
nsCOMPtr<nsPIDOMWindowOuter> newWindow = do_QueryInterface(win);
NS_ASSERTION(newWindow == newDocShell->GetWindow(), "Different windows??");
// The principal of the initial about:blank document gets set up in
@@ -1114,7 +1124,7 @@ nsresult nsWindowWatcher::OpenWindowInternal(
nsCOMPtr<nsIObserverService> obsSvc =
mozilla::services::GetObserverService();
if (obsSvc) {
obsSvc->NotifyObservers(*aResult, "toplevel-window-ready", nullptr);
obsSvc->NotifyObservers(win, "toplevel-window-ready", nullptr);
}
}

2
toolkit/components/windowwatcher/nsWindowWatcher.h
View File

@@ -86,7 +86,7 @@ class nsWindowWatcher : public nsIWindowWatcher,
nsIArray* aArgv, bool aIsPopupSpam,
bool aForceNoOpener, bool aForceNoReferrer,
nsDocShellLoadState* aLoadState,
mozIDOMWindowProxy** aResult);
mozilla::dom::BrowsingContext** aResult);
static nsresult URIfromURL(const char* aURL, mozIDOMWindowProxy* aParent,
nsIURI** aURI);