Skip to content

Commit

Permalink
Bug 1356922 - Part 1: Replace the next TabParent global pointer with …
Browse files Browse the repository at this point in the history 
…per-window/tab next TabParent ID; r=billm,mconley

This patch replaces the usage of sNextTabParent pointer to store the next
PBrowser parent actor to be used by the next frame loader with the
following information:

  * In the case where the content JS has requested a new tab, the ID of the
    next TabParent will be stored on the <xul:browser> element.
  * In the case where the content JS has requested a new window, the ID of
    the next TabParent will be stored on the created nsXULWindow.
  • Loading branch information
@ehsan
ehsan committed Apr 24, 2017
1 parent aab379e commit dce3081
Show file tree
Hide file tree
Showing 17 changed files with 145 additions and 38 deletions.
10 changes: 7 additions & 3 deletions browser/base/content/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5103,7 +5103,8 @@ nsBrowserAccess.prototype = {
_openURIInNewTab(aURI, aReferrer, aReferrerPolicy, aIsPrivate,
aIsExternal, aForceNotRemote = false,
aUserContextId = Ci.nsIScriptSecurityManager.DEFAULT_USER_CONTEXT_ID,
aOpener = null, aTriggeringPrincipal = null) {
aOpener = null, aTriggeringPrincipal = null,
aNextTabParentId = 0) {
let win, needToFocusWin;

// try the current window. if we're in a popup, fall back on the most recent browser window
Expand Down Expand Up @@ -5136,6 +5137,7 @@ nsBrowserAccess.prototype = {
inBackground: loadInBackground,
forceNotRemote: aForceNotRemote,
opener: aOpener,
nextTabParentId: aNextTabParentId,
});
let browser = win.gBrowser.getBrowserForTab(tab);

Expand Down Expand Up @@ -5238,7 +5240,8 @@ nsBrowserAccess.prototype = {
return newWindow;
},

openURIInFrame: function browser_openURIInFrame(aURI, aParams, aWhere, aFlags) {
openURIInFrame: function browser_openURIInFrame(aURI, aParams, aWhere, aFlags,
aNextTabParentId) {
if (aWhere != Ci.nsIBrowserDOMWindow.OPEN_NEWTAB) {
dump("Error: openURIInFrame can only open in new tabs");
return null;
Expand All @@ -5257,7 +5260,8 @@ nsBrowserAccess.prototype = {
aParams.isPrivate,
isExternal, false,
userContextId, null,
aParams.triggeringPrincipal);
aParams.triggeringPrincipal,
aNextTabParentId);
if (browser)
return browser.QueryInterface(Ci.nsIFrameLoaderOwner);

Expand Down
15 changes: 13 additions & 2 deletions browser/base/content/tabbrowser.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1488,6 +1488,7 @@
var aOriginPrincipal;
var aOpener;
var aCreateLazyBrowser;
var aNextTabParentId;
if (arguments.length == 2 &&
typeof arguments[1] == "object" &&
!(arguments[1] instanceof Ci.nsIURI)) {
Expand All @@ -1512,6 +1513,7 @@
aOpener = params.opener;
aIsPrerendered = params.isPrerendered;
aCreateLazyBrowser = params.createLazyBrowser;
aNextTabParentId = params.nextTabParentId;
}
var bgLoad = (aLoadInBackground != null) ? aLoadInBackground :
Expand All @@ -1538,7 +1540,8 @@
originPrincipal: aOriginPrincipal,
sameProcessAsFrameLoader: aSameProcessAsFrameLoader,
opener: aOpener,
isPrerendered: aIsPrerendered });
isPrerendered: aIsPrerendered,
nextTabParentId: aNextTabParentId });
if (!bgLoad)
this.selectedTab = tab;
Expand Down Expand Up @@ -1974,6 +1977,11 @@
b.setAttribute("autoscrollpopup", this._autoScrollPopup.id);
if (aParams.nextTabParentId) {
// Gecko is going to read this attribute and use it.
b.setAttribute("nextTabParentId", aParams.nextTabParentId.toString());
}
if (aParams.sameProcessAsFrameLoader) {
b.sameProcessAsFrameLoader = aParams.sameProcessAsFrameLoader;
}
Expand Down Expand Up @@ -2218,6 +2226,7 @@
var aOpener;
var aCreateLazyBrowser;
var aSkipBackgroundNotify;
var aNextTabParentId;
if (arguments.length == 2 &&
typeof arguments[1] == "object" &&
!(arguments[1] instanceof Ci.nsIURI)) {
Expand Down Expand Up @@ -2245,6 +2254,7 @@
aIsPrerendered = params.isPrerendered;
aCreateLazyBrowser = params.createLazyBrowser;
aSkipBackgroundNotify = params.skipBackgroundNotify;
aNextTabParentId = params.nextTabParentId;
}
// if we're adding tabs, we're past interrupt mode, ditch the owner
Expand Down Expand Up @@ -2353,7 +2363,8 @@
userContextId: aUserContextId,
sameProcessAsFrameLoader: aSameProcessAsFrameLoader,
opener: aOpener,
isPrerendered: aIsPrerendered });
isPrerendered: aIsPrerendered,
nextTabParentId: aNextTabParentId });
}
t.linkedBrowser = b;
Expand Down
19 changes: 18 additions & 1 deletion dom/base/nsFrameLoader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2957,9 +2957,26 @@ nsFrameLoader::TryRemoteBrowser()
nsresult rv = GetNewTabContext(&context);
NS_ENSURE_SUCCESS(rv, false);

uint64_t nextTabParentId = 0;
if (mOwnerContent) {
nsAutoString nextTabParentIdAttr;
mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nextTabParentId,
nextTabParentIdAttr);
nextTabParentId = strtoull(NS_ConvertUTF16toUTF8(nextTabParentIdAttr).get(),
nullptr, 10);

// We may be in a window that was just opened, so try the
// nsIBrowserDOMWindow API as a backup.
if (!nextTabParentId && window) {
Unused << window->GetNextTabParentId(&nextTabParentId);
}
}

nsCOMPtr<Element> ownerElement = mOwnerContent;
mRemoteBrowser = ContentParent::CreateBrowser(context, ownerElement,
openerContentParent, sameTabGroupAs);
openerContentParent,
sameTabGroupAs,
nextTabParentId);
if (!mRemoteBrowser) {
return false;
}
Expand Down
1 change: 1 addition & 0 deletions dom/base/nsGkAtomList.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ GK_ATOM(never, "never")
GK_ATOM(_new, "new")
GK_ATOM(newline, "newline")
GK_ATOM(nextBidi, "NextBidi")
GK_ATOM(nextTabParentId, "nextTabParentId")
GK_ATOM(no, "no")
GK_ATOM(noautofocus, "noautofocus")
GK_ATOM(noautohide, "noautohide")
Expand Down
3 changes: 2 additions & 1 deletion dom/interfaces/base/nsIBrowserDOMWindow.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ interface nsIBrowserDOMWindow : nsISupports
// See bug 537428
*/
nsIFrameLoaderOwner openURIInFrame(in nsIURI aURI, in nsIOpenURIInFrameParams params,
in short aWhere, in long aFlags);
in short aWhere, in long aFlags,
in unsigned long long aNextTabParentId);

/**
* @param aWindow the window to test.
Expand Down
32 changes: 25 additions & 7 deletions dom/ipc/ContentParent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ StaticAutoPtr<LinkedList<ContentParent> > ContentParent::sContentParents;
#if defined(XP_LINUX) && defined(MOZ_CONTENT_SANDBOX)
UniquePtr<SandboxBrokerPolicyFactory> ContentParent::sSandboxBrokerPolicyFactory;
#endif
uint64_t ContentParent::sNextTabParentId = 0;
nsDataHashtable<nsUint64HashKey, TabParent*> ContentParent::sNextTabParents;

// This is true when subprocess launching is enabled. This is the
// case between StartUp() and ShutDown() or JoinAllSubprocesses().
Expand Down Expand Up @@ -1152,17 +1154,23 @@ ContentParent::RecvFindPlugins(const uint32_t& aPluginEpoch,
ContentParent::CreateBrowser(const TabContext& aContext,
Element* aFrameElement,
ContentParent* aOpenerContentParent,
TabParent* aSameTabGroupAs)
TabParent* aSameTabGroupAs,
uint64_t aNextTabParentId)
{
PROFILER_LABEL_FUNC(js::ProfileEntry::Category::OTHER);

if (!sCanLaunchSubprocesses) {
return nullptr;
}

if (TabParent* parent = TabParent::GetNextTabParent()) {
parent->SetOwnerElement(aFrameElement);
return parent;
if (aNextTabParentId) {
if (TabParent* parent =
sNextTabParents.GetAndRemove(aNextTabParentId).valueOr(nullptr)) {
MOZ_ASSERT(!parent->GetOwnerElement(),
"Shouldn't have an owner elemnt before");
parent->SetOwnerElement(aFrameElement);
return parent;
}
}

ProcessPriority initialPriority = GetInitialProcessPriority(aFrameElement);
Expand Down Expand Up @@ -4462,6 +4470,7 @@ ContentParent::CommonCreateWindow(PBrowserParent* aThisTab,
const nsCString& aBaseURI,
const OriginAttributes& aOpenerOriginAttributes,
const float& aFullZoom,
uint64_t aNextTabParentId,
nsresult& aResult,
nsCOMPtr<nsITabParent>& aNewTabParent,
bool* aWindowIsNew)
Expand Down Expand Up @@ -4544,6 +4553,7 @@ ContentParent::CommonCreateWindow(PBrowserParent* aThisTab,
nsCOMPtr<nsIFrameLoaderOwner> frameLoaderOwner;
aResult = browserDOMWin->OpenURIInFrame(aURIToLoad, params, openLocation,
nsIBrowserDOMWindow::OPEN_NEW,
aNextTabParentId,
getter_AddRefs(frameLoaderOwner));
if (NS_SUCCEEDED(aResult) && frameLoaderOwner) {
RefPtr<nsFrameLoader> frameLoader = frameLoaderOwner->GetFrameLoader();
Expand All @@ -4565,6 +4575,7 @@ ContentParent::CommonCreateWindow(PBrowserParent* aThisTab,

aResult = pwwatch->OpenWindowWithTabParent(aSetOpener ? thisTabParent : nullptr,
aFeatures, aCalledFromJS, aFullZoom,
aNextTabParentId,
getter_AddRefs(aNewTabParent));
if (NS_WARN_IF(NS_FAILED(aResult))) {
return IPC_OK();
Expand Down Expand Up @@ -4630,14 +4641,17 @@ ContentParent::RecvCreateWindow(PBrowserParent* aThisTab,
// we must have an opener.
newTab->SetHasContentOpener(true);

TabParent::AutoUseNewTab aunt(newTab, aWindowIsNew, aURLToLoad);
TabParent::AutoUseNewTab aunt(newTab, aURLToLoad);
const uint64_t nextTabParentId = ++sNextTabParentId;
sNextTabParents.Put(nextTabParentId, newTab);

nsCOMPtr<nsITabParent> newRemoteTab;
mozilla::ipc::IPCResult ipcResult =
CommonCreateWindow(aThisTab, /* aSetOpener = */ true, aChromeFlags,
aCalledFromJS, aPositionSpecified, aSizeSpecified,
nullptr, aFeatures, aBaseURI, aOpenerOriginAttributes,
aFullZoom, *aResult, newRemoteTab, aWindowIsNew);
aFullZoom, nextTabParentId, *aResult,
newRemoteTab, aWindowIsNew);
if (!ipcResult) {
return ipcResult;
}
Expand All @@ -4646,6 +4660,9 @@ ContentParent::RecvCreateWindow(PBrowserParent* aThisTab,
return IPC_OK();
}

if (sNextTabParents.GetAndRemove(nextTabParentId).valueOr(nullptr)) {
*aWindowIsNew = false;
}
MOZ_ASSERT(TabParent::GetFrom(newRemoteTab) == newTab);

newTab->SwapFrameScriptsFrom(*aFrameScripts);
Expand Down Expand Up @@ -4681,7 +4698,8 @@ ContentParent::RecvCreateWindowInDifferentProcess(
CommonCreateWindow(aThisTab, /* aSetOpener = */ false, aChromeFlags,
aCalledFromJS, aPositionSpecified, aSizeSpecified,
uriToLoad, aFeatures, aBaseURI, aOpenerOriginAttributes,
aFullZoom, rv, newRemoteTab, &windowIsNew);
aFullZoom, /* aNextTabParentId = */ 0, rv,
newRemoteTab, &windowIsNew);
if (!ipcResult) {
return ipcResult;
}
Expand Down
7 changes: 6 additions & 1 deletion dom/ipc/ContentParent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ class ContentParent final : public PContentParent
CreateBrowser(const TabContext& aContext,
Element* aFrameElement,
ContentParent* aOpenerContentParent,
TabParent* aSameTabGroupAs);
TabParent* aSameTabGroupAs,
uint64_t aNextTabParentId);

static void GetAll(nsTArray<ContentParent*>& aArray);

Expand Down Expand Up @@ -719,6 +720,7 @@ class ContentParent final : public PContentParent
const nsCString& aBaseURI,
const OriginAttributes& aOpenerOriginAttributes,
const float& aFullZoom,
uint64_t aNextTabParentId,
nsresult& aResult,
nsCOMPtr<nsITabParent>& aNewTabParent,
bool* aWindowIsNew);
Expand Down Expand Up @@ -1274,6 +1276,9 @@ class ContentParent final : public PContentParent
#ifdef MOZ_CRASHREPORTER
UniquePtr<mozilla::ipc::CrashReporterHost> mCrashReporter;
#endif

static uint64_t sNextTabParentId;
static nsDataHashtable<nsUint64HashKey, TabParent*> sNextTabParents;
};

} // namespace dom
Expand Down
13 changes: 2 additions & 11 deletions dom/ipc/TabParent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,13 +778,11 @@ class TabParent final : public PBrowserParent
struct MOZ_STACK_CLASS TabParent::AutoUseNewTab final
{
public:
AutoUseNewTab(TabParent* aNewTab, bool* aWindowIsNew, nsCString* aURLToLoad)
: mNewTab(aNewTab), mWindowIsNew(aWindowIsNew), mURLToLoad(aURLToLoad)
AutoUseNewTab(TabParent* aNewTab, nsCString* aURLToLoad)
: mNewTab(aNewTab), mURLToLoad(aURLToLoad)
{
MOZ_ASSERT(!TabParent::sNextTabParent);
MOZ_ASSERT(!aNewTab->mCreatingWindow);

TabParent::sNextTabParent = aNewTab;
aNewTab->mCreatingWindow = true;
aNewTab->mDelayedURL.Truncate();
}
Expand All @@ -793,17 +791,10 @@ struct MOZ_STACK_CLASS TabParent::AutoUseNewTab final
{
mNewTab->mCreatingWindow = false;
*mURLToLoad = mNewTab->mDelayedURL;

if (TabParent::sNextTabParent) {
MOZ_ASSERT(TabParent::sNextTabParent == mNewTab);
TabParent::sNextTabParent = nullptr;
*mWindowIsNew = false;
}
}

private:
TabParent* mNewTab;
bool* mWindowIsNew;
nsCString* mURLToLoad;
};

Expand Down
10 changes: 8 additions & 2 deletions mobile/android/chrome/content/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3440,9 +3440,15 @@ nsBrowserAccess.prototype = {
return browser ? browser.contentWindow : null;
},

openURIInFrame: function browser_openURIInFrame(aURI, aParams, aWhere, aFlags) {
openURIInFrame: function browser_openURIInFrame(aURI, aParams, aWhere, aFlags,
aNextTabParentId) {
// We currently ignore aNextTabParentId on mobile. This needs to change
// when Fennec starts to support e10s. Assertions will fire if this code
// isn't fixed by then.
let browser = this._getBrowser(aURI, null, aWhere, aFlags);
return browser ? browser.QueryInterface(Ci.nsIFrameLoaderOwner) : null;
if (browser)
return browser.QueryInterface(Ci.nsIFrameLoaderOwner);
return null;
},

isTabContentWindow: function(aWindow) {
Expand Down
10 changes: 8 additions & 2 deletions toolkit/components/startup/nsAppStartup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ nsAppStartup::CreateChromeWindow(nsIWebBrowserChrome *aParent,
nsIWebBrowserChrome **_retval)
{
bool cancel;
return CreateChromeWindow2(aParent, aChromeFlags, nullptr, nullptr, &cancel, _retval);
return CreateChromeWindow2(aParent, aChromeFlags, nullptr, nullptr, 0, &cancel, _retval);
}


Expand All @@ -633,6 +633,7 @@ nsAppStartup::CreateChromeWindow2(nsIWebBrowserChrome *aParent,
uint32_t aChromeFlags,
nsITabParent *aOpeningTab,
mozIDOMWindowProxy* aOpener,
uint64_t aNextTabParentId,
bool *aCancel,
nsIWebBrowserChrome **_retval)
{
Expand All @@ -652,10 +653,15 @@ nsAppStartup::CreateChromeWindow2(nsIWebBrowserChrome *aParent,
NS_ASSERTION(xulParent, "window created using non-XUL parent. that's unexpected, but may work.");

if (xulParent)
xulParent->CreateNewWindow(aChromeFlags, aOpeningTab, aOpener, getter_AddRefs(newWindow));
xulParent->CreateNewWindow(aChromeFlags, aOpeningTab, aOpener,
aNextTabParentId,
getter_AddRefs(newWindow));
// And if it fails, don't try again without a parent. It could fail
// intentionally (bug 115969).
} else { // try using basic methods:
MOZ_RELEASE_ASSERT(aNextTabParentId == 0,
"Unexpected aNextTabParentId, we shouldn't ever have a next actor ID without a parent");

/* You really shouldn't be making dependent windows without a parent.
But unparented modal (and therefore dependent) windows happen
in our codebase, so we allow it after some bellyaching: */
Expand Down
3 changes: 3 additions & 0 deletions toolkit/components/windowcreator/nsIWindowCreator2.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ interface nsIWindowCreator2 : nsIWindowCreator {
window. Can be nullptr.
@param aOpener The window which is trying to open this new chrome window.
Can be nullptr
@param aNextTabParentId The integer ID of the next tab parent actor to use.
0 means there is no next tab parent ID to use.
@param cancel Return |true| to reject window creation. If true the
implementation has determined the window should not
be created at all. The caller should not default
Expand All @@ -47,6 +49,7 @@ interface nsIWindowCreator2 : nsIWindowCreator {
in uint32_t chromeFlags,
in nsITabParent aOpeningTab,
in mozIDOMWindowProxy aOpener,
in unsigned long long aNextTabParentId,
out boolean cancel);

/**
Expand Down
6 changes: 5 additions & 1 deletion toolkit/components/windowwatcher/nsPIWindowWatcher.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,18 @@ interface nsPIWindowWatcher : nsISupports
* @param aOpenerFullZoom
* The current zoom multiplier for the opener tab. This is then
* applied to the newly opened window.
* @param aNextTabParentId
* The integer ID for the next tab parent actor.
* 0 means there is no next tab parent actor to use.
*
* @return the nsITabParent of the initial browser for the newly opened
* window.
*/
nsITabParent openWindowWithTabParent(in nsITabParent aOpeningTab,
in ACString aFeatures,
in boolean aCalledFromJS,
in float aOpenerFullZoom);
in float aOpenerFullZoom,
in unsigned long long aNextTabParentId);

/**
* Find a named docshell tree item amongst all windows registered
Expand Down
Loading

0 comments on commit dce3081

Please sign in to comment.