Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

Commit

Permalink
Back out fbdb3104c9e5, d2fe54ae00a8, d378362cbe01, fe623d60bea1 (bug …
Browse files Browse the repository at this point in the history
…769254) on suspicion of causing Windows debug mochitest-plain-3 timeouts
  • Loading branch information
mbrubeck committed Jul 24, 2012
1 parent c3e7952 commit d1ea65a
Show file tree
Hide file tree
Showing 17 changed files with 169 additions and 340 deletions.
13 changes: 5 additions & 8 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8307,19 +8307,16 @@ nsDocShell::InternalLoad(nsIURI * aURI,

bool isNewWindow = false;
if (!targetDocShell) {
nsCOMPtr<nsPIDOMWindow> win =
nsCOMPtr<nsIDOMWindow> win =
do_GetInterface(GetAsSupports(this));
NS_ENSURE_TRUE(win, NS_ERROR_NOT_AVAILABLE);

nsDependentString name(aWindowTarget);
nsCOMPtr<nsIDOMWindow> newWin;
nsCAutoString spec;
if (aURI)
aURI->GetSpec(spec);
rv = win->OpenNoNavigate(NS_ConvertUTF8toUTF16(spec),
name, // window name
EmptyString(), // Features
getter_AddRefs(newWin));
rv = win->Open(EmptyString(), // URL to load
name, // window name
EmptyString(), // Features
getter_AddRefs(newWin));

// In some cases the Open call doesn't actually result in a new
// window being opened. We can detect these cases by examining the
Expand Down
72 changes: 19 additions & 53 deletions dom/base/nsGlobalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5866,7 +5866,6 @@ nsGlobalWindow::Open(const nsAString& aUrl, const nsAString& aName,
false, // aContentModal
true, // aCalledNoScript
false, // aDoJSFixups
true, // aNavigate
nsnull, nsnull, // No args
GetPrincipal(), // aCalleePrincipal
nsnull, // aJSCallerContext
Expand All @@ -5882,7 +5881,6 @@ nsGlobalWindow::OpenJS(const nsAString& aUrl, const nsAString& aName,
false, // aContentModal
false, // aCalledNoScript
true, // aDoJSFixups
true, // aNavigate
nsnull, nsnull, // No args
GetPrincipal(), // aCalleePrincipal
nsContentUtils::GetCurrentJSContext(), // aJSCallerContext
Expand All @@ -5901,33 +5899,12 @@ nsGlobalWindow::OpenDialog(const nsAString& aUrl, const nsAString& aName,
false, // aContentModal
true, // aCalledNoScript
false, // aDoJSFixups
true, // aNavigate
nsnull, aExtraArgument, // Arguments
GetPrincipal(), // aCalleePrincipal
nsnull, // aJSCallerContext
_retval);
}

// Like Open, but passes aNavigate=false.
/* virtual */ nsresult
nsGlobalWindow::OpenNoNavigate(const nsAString& aUrl,
const nsAString& aName,
const nsAString& aOptions,
nsIDOMWindow **_retval)
{
return OpenInternal(aUrl, aName, aOptions,
false, // aDialog
false, // aContentModal
true, // aCalledNoScript
false, // aDoJSFixups
false, // aNavigate
nsnull, nsnull, // No args
GetPrincipal(), // aCalleePrincipal
nsnull, // aJSCallerContext
_retval);

}

NS_IMETHODIMP
nsGlobalWindow::OpenDialog(const nsAString& aUrl, const nsAString& aName,
const nsAString& aOptions, nsIDOMWindow** _retval)
Expand Down Expand Up @@ -5968,7 +5945,6 @@ nsGlobalWindow::OpenDialog(const nsAString& aUrl, const nsAString& aName,
false, // aContentModal
false, // aCalledNoScript
false, // aDoJSFixups
true, // aNavigate
argvArray, nsnull, // Arguments
GetPrincipal(), // aCalleePrincipal
cx, // aJSCallerContext
Expand Down Expand Up @@ -7212,7 +7188,6 @@ nsGlobalWindow::ShowModalDialog(const nsAString& aURI, nsIVariant *aArgs,
true, // aContentModal
true, // aCalledNoScript
true, // aDoJSFixups
true, // aNavigate
nsnull, aArgs, // args
GetPrincipal(), // aCalleePrincipal
nsnull, // aJSCallerContext
Expand Down Expand Up @@ -9131,17 +9106,16 @@ nsresult
nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
const nsAString& aOptions, bool aDialog,
bool aContentModal, bool aCalledNoScript,
bool aDoJSFixups, bool aNavigate,
nsIArray *argv,
bool aDoJSFixups, nsIArray *argv,
nsISupports *aExtraArgument,
nsIPrincipal *aCalleePrincipal,
JSContext *aJSCallerContext,
nsIDOMWindow **aReturn)
{
FORWARD_TO_OUTER(OpenInternal, (aUrl, aName, aOptions, aDialog,
aContentModal, aCalledNoScript, aDoJSFixups,
aNavigate, argv, aExtraArgument,
aCalleePrincipal, aJSCallerContext, aReturn),
argv, aExtraArgument, aCalleePrincipal,
aJSCallerContext, aReturn),
NS_ERROR_NOT_INITIALIZED);

#ifdef DEBUG
Expand All @@ -9156,9 +9130,6 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
NS_PRECONDITION(!aJSCallerContext || !aCalledNoScript,
"Shouldn't have caller context when called noscript");

// Calls to window.open from script should navigate.
MOZ_ASSERT(aCalledNoScript || aNavigate);

*aReturn = nsnull;

nsCOMPtr<nsIWebBrowserChrome> chrome;
Expand Down Expand Up @@ -9186,13 +9157,10 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
if (!aUrl.IsEmpty()) {
AppendUTF16toUTF8(aUrl, url);

// It's safe to skip the security check below if we're not a dialog
// because window.openDialog is not callable from content script. See bug
// 56851.
//
// If we're not navigating, we assume that whoever *does* navigate the
// window will do a security check of their own.
if (url.get() && !aDialog && aNavigate)
/* Check whether the URI is allowed, but not for dialogs --
see bug 56851. The security of this function depends on
window.openDialog being inaccessible from web scripts */
if (url.get() && !aDialog)
rv = SecurityCheckURL(url.get());
}

Expand Down Expand Up @@ -9233,22 +9201,22 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
const char *options_ptr = aOptions.IsEmpty() ? nsnull : options.get();
const char *name_ptr = aName.IsEmpty() ? nsnull : name.get();

nsCOMPtr<nsPIWindowWatcher> pwwatch(do_QueryInterface(wwatch));
NS_ENSURE_STATE(pwwatch);

{
// Reset popup state while opening a window to prevent the
// current state from being active the whole time a modal
// dialog is open.
nsAutoPopupStatePusher popupStatePusher(openAbused, true);

if (!aCalledNoScript) {
// We asserted at the top of this function that aNavigate is true for
// !aCalledNoScript.
rv = pwwatch->OpenWindow2(this, url.get(), name_ptr, options_ptr,
/* aCalledFromScript = */ true,
aDialog, aNavigate, argv,
getter_AddRefs(domReturn));
nsCOMPtr<nsPIWindowWatcher> pwwatch(do_QueryInterface(wwatch));
NS_ASSERTION(pwwatch,
"Unable to open windows from JS because window watcher "
"is broken");
NS_ENSURE_TRUE(pwwatch, NS_ERROR_UNEXPECTED);

rv = pwwatch->OpenWindowJS(this, url.get(), name_ptr, options_ptr,
aDialog, argv,
getter_AddRefs(domReturn));
} else {
// Push a null JSContext 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
Expand All @@ -9264,11 +9232,9 @@ nsGlobalWindow::OpenInternal(const nsAString& aUrl, const nsAString& aName,
rv = stack->Push(nsnull);
NS_ENSURE_SUCCESS(rv, rv);
}

rv = pwwatch->OpenWindow2(this, url.get(), name_ptr, options_ptr,
/* aCalledFromScript = */ false,
aDialog, aNavigate, aExtraArgument,
getter_AddRefs(domReturn));

rv = wwatch->OpenWindow(this, url.get(), name_ptr, options_ptr,
aExtraArgument, getter_AddRefs(domReturn));

if (stack) {
JSContext* cx;
Expand Down
60 changes: 20 additions & 40 deletions dom/base/nsGlobalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -674,56 +674,37 @@ class nsGlobalWindow : public nsPIDOMWindow,
}

// Window Control Functions

virtual nsresult
OpenNoNavigate(const nsAString& aUrl,
const nsAString& aName,
const nsAString& aOptions,
nsIDOMWindow **_retval);

/**
* @param aUrl the URL we intend to load into the window. If aNavigate is
* true, we'll actually load this URL into the window. Otherwise,
* aUrl is advisory; OpenInternal will not load the URL into the
* new window.
*
* @param aURL the URL to load in the new window
* @param aName the name to use for the new window
*
* @param aOptions the window options to use for the new window
*
* @param aDialog true when called from variants of OpenDialog. If this is
* true, this method will skip popup blocking checks. The aDialog
* argument is passed on to the window watcher.
*
* true, this method will skip popup blocking checks. The
* aDialog argument is passed on to the window watcher.
* @param aCalledNoScript true when called via the [noscript] open()
* and openDialog() methods. When this is true, we do NOT want to use
* the JS stack for things like caller determination.
*
* and openDialog() methods. When this is true, we do
* NOT want to use the JS stack for things like caller
* determination.
* @param aDoJSFixups true when this is the content-accessible JS version of
* window opening. When true, popups do not cause us to throw, we save
* the caller's principal in the new window for later consumption, and
* we make sure that there is a document in the newly-opened window.
* Note that this last will only be done if the newly-opened window is
* non-chrome.
*
* @param aNavigate true if we should navigate to the provided URL, false
* otherwise. When aNavigate is false, we also skip our can-load
* security check, on the assumption that whoever *actually* loads this
* page will do their own security check.
*
* window opening. When true, popups do not cause us to
* throw, we save the caller's principal in the new window
* for later consumption, and we make sure that there is a
* document in the newly-opened window. Note that this
* last will only be done if the newly-opened window is
* non-chrome.
* @param argv The arguments to pass to the new window. The first
* three args, if present, will be aUrl, aName, and aOptions. So this
* param only matters if there are more than 3 arguments.
*
* three args, if present, will be aURL, aName, and aOptions. So
* this param only matters if there are more than 3 arguments.
* @param argc The number of arguments in argv.
*
* @param aExtraArgument Another way to pass arguments in. This is mutually
* exclusive with the argv/argc approach.
*
* exclusive with the argv/argc approach.
* @param aJSCallerContext The calling script's context. This must be nsnull
* when aCalledNoScript is true.
*
* when aCalledNoScript is true.
* @param aReturn [out] The window that was opened, if any.
*
* @note that the boolean args are const because the function shouldn't be
* messing with them. That also makes it easier for the compiler to sort out
* its build warning stuff.
*/
NS_HIDDEN_(nsresult) OpenInternal(const nsAString& aUrl,
const nsAString& aName,
Expand All @@ -732,7 +713,6 @@ class nsGlobalWindow : public nsPIDOMWindow,
bool aContentModal,
bool aCalledNoScript,
bool aDoJSFixups,
bool aNavigate,
nsIArray *argv,
nsISupports *aExtraArgument,
nsIPrincipal *aCalleePrincipal,
Expand Down
11 changes: 2 additions & 9 deletions dom/base/nsPIDOMWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class nsIArray;
class nsPIWindowRoot;

#define NS_PIDOMWINDOW_IID \
{0x66660102, 0xd875, 0x47e2, \
{0xa1, 0xf7, 0x12, 0xbc, 0x83, 0xc9, 0x93, 0xa9}}
{ 0x0c4d0b84, 0xb524, 0x4572, \
{ 0x8e, 0xd1, 0x7f, 0x78, 0x14, 0x7c, 0x4d, 0xf1 } }

class nsPIDOMWindow : public nsIDOMWindowInternal
{
Expand Down Expand Up @@ -602,13 +602,6 @@ class nsPIDOMWindow : public nsIDOMWindowInternal
*/
virtual bool IsPartOfApp() = 0;

/**
* Like nsIDOMWindow::Open, except that we don't navigate to the given URL.
*/
virtual nsresult
OpenNoNavigate(const nsAString& aUrl, const nsAString& aName,
const nsAString& aOptions, nsIDOMWindow **_retval) = 0;

protected:
// The nsPIDOMWindow constructor. The aOuterWindow argument should
// be null if and only if the created window itself is an outer
Expand Down
4 changes: 1 addition & 3 deletions dom/browser-element/BrowserElementParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,7 @@ BrowserElementParent::OpenWindowInProcess(nsIDOMWindow* aOpenerWindow,
NS_ENSURE_TRUE(popupFrameElement, false);

nsCAutoString spec;
if (aURI) {
aURI->GetSpec(spec);
}
aURI->GetSpec(spec);
bool dispatchSucceeded =
DispatchOpenWindowEvent(openerFrameElement, popupFrameElement,
NS_ConvertUTF8toUTF16(spec),
Expand Down
5 changes: 0 additions & 5 deletions dom/browser-element/BrowserElementParent.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ class BrowserElementParent
* set aPopupTabParent's frame element to event.detail.frameElement.
* Otherwise, we return false.
*
* @param aURL the absolute URL the new window should load. The empty string
* is allowed indicates we shouldn't load anything.
* @param aOpenerTabParent the TabParent whose TabChild called window.open.
* @param aPopupTabParent the TabParent inside which the opened window will
* live.
Expand All @@ -77,9 +75,6 @@ class BrowserElementParent
*
* (These parameter types are silly, but they match what our caller has in
* hand. Feel free to add an override, if they are inconvenient to you.)
*
* @param aURI the URI the new window should load. May be null, which
* indicates that we shouldn't load anything.
*/
static bool
OpenWindowInProcess(nsIDOMWindow* aOpenerWindow,
Expand Down
4 changes: 0 additions & 4 deletions dom/browser-element/mochitest/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ MOCHITEST_FILES = \
test_browserElement_inproc_AlertInFrame.html \
file_browserElement_AlertInFrame.html \
file_browserElement_AlertInFrame_Inner.html \
browserElement_TargetBlank.js \
test_browserElement_inproc_TargetBlank.html \
file_browserElement_TargetBlank.html \
browserElement_TargetTop.js \
test_browserElement_inproc_TargetTop.html \
file_browserElement_TargetTop.html \
Expand Down Expand Up @@ -133,7 +130,6 @@ MOCHITEST_FILES += \
test_browserElement_oop_XFrameOptionsSameOrigin.html \
test_browserElement_oop_Alert.html \
test_browserElement_oop_AlertInFrame.html \
test_browserElement_oop_TargetBlank.html \
test_browserElement_oop_TargetTop.html \
test_browserElement_oop_PromptCheck.html \
test_browserElement_oop_PromptConfirm.html \
Expand Down
26 changes: 0 additions & 26 deletions dom/browser-element/mochitest/browserElement_TargetBlank.js

This file was deleted.

18 changes: 0 additions & 18 deletions dom/browser-element/mochitest/file_browserElement_TargetBlank.html

This file was deleted.

Loading

0 comments on commit d1ea65a

Please sign in to comment.