Skip to content

Commit

Permalink
Bug 1354249 - Use same TabGroup as original tab for print preview (r=…
Browse files Browse the repository at this point in the history 
…mystor)

MozReview-Commit-ID: 5wmjLgq2j5m
  • Loading branch information
@bill-mccloskey
bill-mccloskey committed Apr 13, 2017
1 parent 6aea6a4 commit 5c60972
Show file tree
Hide file tree
Showing 22 changed files with 163 additions and 60 deletions.
19 changes: 12 additions & 7 deletions dom/base/TabGroup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,22 @@ TabGroup::GetChromeTabGroup()
}

/* static */ TabGroup*
TabGroup::GetFromWindowActor(mozIDOMWindowProxy* aWindow)
TabGroup::GetFromWindow(mozIDOMWindowProxy* aWindow)
{
MOZ_RELEASE_ASSERT(NS_IsMainThread());

TabChild* tabChild = TabChild::GetFrom(aWindow);
if (!tabChild) {
return nullptr;
if (TabChild* tabChild = TabChild::GetFrom(aWindow)) {
return tabChild->TabGroup();
}

return nullptr;
}

/* static */ TabGroup*
TabGroup::GetFromActor(TabChild* aTabChild)
{
MOZ_RELEASE_ASSERT(NS_IsMainThread());

ContentChild* cc = ContentChild::GetSingleton();
nsCOMPtr<nsIEventTarget> target = cc->GetActorEventTarget(tabChild);
nsCOMPtr<nsIEventTarget> target = cc->GetActorEventTarget(aTabChild);
if (!target) {
return nullptr;
}
Expand Down
14 changes: 7 additions & 7 deletions dom/base/TabGroup.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,13 @@ class TabGroup final : public SchedulerGroup
static TabGroup*
GetChromeTabGroup();

// Checks if the PBrowserChild associated with aWindow already has a TabGroup
// assigned to it in IPDL. Returns this TabGroup if it does. This could happen
// if the parent process created the PBrowser and we needed to assign a
// TabGroup immediately upon receiving the IPDL message. This method is main
// thread only.
static TabGroup*
GetFromWindowActor(mozIDOMWindowProxy* aWindow);
// Checks if the TabChild already has a TabGroup assigned to it in
// IPDL. Returns this TabGroup if it does. This could happen if the parent
// process created the PBrowser and we needed to assign a TabGroup immediately
// upon receiving the IPDL message. This method is main thread only.
static TabGroup* GetFromActor(TabChild* aTabChild);

static TabGroup* GetFromWindow(mozIDOMWindowProxy* aWindow);

explicit TabGroup(bool aIsChrome = false);

Expand Down
20 changes: 12 additions & 8 deletions dom/base/nsFrameLoader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2833,28 +2833,30 @@ nsFrameLoader::SetClampScrollPosition(bool aClamp)
}

static
ContentParent*
Tuple<ContentParent*, TabParent*>
GetContentParent(Element* aBrowser)
{
using ReturnTuple = Tuple<ContentParent*, TabParent*>;

nsCOMPtr<nsIBrowser> browser = do_QueryInterface(aBrowser);
if (!browser) {
return nullptr;
return ReturnTuple(nullptr, nullptr);
}

nsCOMPtr<nsIFrameLoader> otherLoader;
browser->GetSameProcessAsFrameLoader(getter_AddRefs(otherLoader));
if (!otherLoader) {
return nullptr;
return ReturnTuple(nullptr, nullptr);
}

TabParent* tabParent = TabParent::GetFrom(otherLoader);
if (tabParent &&
tabParent->Manager() &&
tabParent->Manager()->IsContentParent()) {
return tabParent->Manager()->AsContentParent();
return MakeTuple(tabParent->Manager()->AsContentParent(), tabParent);
}

return nullptr;
return ReturnTuple(nullptr, nullptr);
}

bool
Expand Down Expand Up @@ -2889,7 +2891,8 @@ nsFrameLoader::TryRemoteBrowser()
}

TabParent* openingTab = TabParent::GetFrom(parentDocShell->GetOpener());
ContentParent* openerContentParent = nullptr;
RefPtr<ContentParent> openerContentParent;
RefPtr<TabParent> sameTabGroupAs;

if (openingTab &&
openingTab->Manager() &&
Expand Down Expand Up @@ -2933,7 +2936,7 @@ nsFrameLoader::TryRemoteBrowser()
}

// Try to get the related content parent from our browser element.
openerContentParent = GetContentParent(mOwnerContent);
Tie(openerContentParent, sameTabGroupAs) = GetContentParent(mOwnerContent);
}

uint32_t chromeFlags = 0;
Expand All @@ -2955,7 +2958,8 @@ nsFrameLoader::TryRemoteBrowser()
NS_ENSURE_SUCCESS(rv, false);

nsCOMPtr<Element> ownerElement = mOwnerContent;
mRemoteBrowser = ContentParent::CreateBrowser(context, ownerElement, openerContentParent);
mRemoteBrowser = ContentParent::CreateBrowser(context, ownerElement,
openerContentParent, sameTabGroupAs);
if (!mRemoteBrowser) {
return false;
}
Expand Down
9 changes: 4 additions & 5 deletions dom/base/nsGlobalWindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15005,15 +15005,14 @@ nsGlobalWindow::TabGroupOuter()
} else if (parent) {
toJoin = parent->TabGroup();
} else {
// If the tab was created by the parent process, the IPC code may have
// already created a TabGroup for us. Fetch it in that case.
toJoin = TabGroup::GetFromWindowActor(AsOuter());
toJoin = TabGroup::GetFromWindow(AsOuter());
}

#ifdef DEBUG
// Make sure that, if we have a tab group from the actor, it matches the one
// we're planning to join.
mozilla::dom::TabGroup* actorTabGroup = TabGroup::GetFromWindowActor(AsOuter());
MOZ_ASSERT_IF(actorTabGroup, actorTabGroup == toJoin);
mozilla::dom::TabGroup* testGroup = TabGroup::GetFromWindow(AsOuter());
MOZ_ASSERT_IF(testGroup, testGroup == toJoin);
#endif

mTabGroup = mozilla::dom::TabGroup::Join(AsOuter(), toJoin);
Expand Down
6 changes: 6 additions & 0 deletions dom/ipc/ContentBridgeChild.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,15 @@ ContentBridgeChild::SendPMemoryStreamConstructor(const uint64_t& aSize)
bool
ContentBridgeChild::SendPBrowserConstructor(PBrowserChild* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser)
{
return PContentBridgeChild::SendPBrowserConstructor(aActor,
aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand Down Expand Up @@ -128,12 +130,14 @@ ContentBridgeChild::DeallocPJavaScriptChild(PJavaScriptChild *child)

PBrowserChild*
ContentBridgeChild::AllocPBrowserChild(const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext &aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser)
{
return nsIContentChild::AllocPBrowserChild(aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand All @@ -149,13 +153,15 @@ ContentBridgeChild::DeallocPBrowserChild(PBrowserChild* aChild)
mozilla::ipc::IPCResult
ContentBridgeChild::RecvPBrowserConstructor(PBrowserChild* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser)
{
return nsIContentChild::RecvPBrowserConstructor(aActor,
aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand Down
3 changes: 3 additions & 0 deletions dom/ipc/ContentBridgeChild.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class ContentBridgeChild final : public PContentBridgeChild

virtual bool SendPBrowserConstructor(PBrowserChild* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
Expand All @@ -67,13 +68,15 @@ class ContentBridgeChild final : public PContentBridgeChild
virtual ~ContentBridgeChild();

virtual PBrowserChild* AllocPBrowserChild(const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser) override;
virtual bool DeallocPBrowserChild(PBrowserChild*) override;
virtual mozilla::ipc::IPCResult RecvPBrowserConstructor(PBrowserChild* aCctor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
Expand Down
4 changes: 4 additions & 0 deletions dom/ipc/ContentBridgeParent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,15 @@ ContentBridgeParent::SendPBlobConstructor(PBlobParent* actor,
PBrowserParent*
ContentBridgeParent::SendPBrowserConstructor(PBrowserParent* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser)
{
return PContentBridgeParent::SendPBrowserConstructor(aActor,
aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand Down Expand Up @@ -155,12 +157,14 @@ ContentBridgeParent::DeallocPJavaScriptParent(PJavaScriptParent *parent)

PBrowserParent*
ContentBridgeParent::AllocPBrowserParent(const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext &aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser)
{
return nsIContentParent::AllocPBrowserParent(aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand Down
2 changes: 2 additions & 0 deletions dom/ipc/ContentBridgeParent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class ContentBridgeParent : public PContentBridgeParent
virtual PBrowserParent*
SendPBrowserConstructor(PBrowserParent* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
Expand Down Expand Up @@ -124,6 +125,7 @@ class ContentBridgeParent : public PContentBridgeParent

virtual PBrowserParent*
AllocPBrowserParent(const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext &aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
Expand Down
65 changes: 51 additions & 14 deletions dom/ipc/ContentChild.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -775,8 +775,19 @@ ContentChild::ProvideWindowCommon(TabChild* aTabOpener,
GetID(),
&tabId);

// We need to assign a TabGroup to the PBrowser actor before we send it to the
// parent. Otherwise, the parent could send messages to us before we have a
// proper TabGroup for that actor.
RefPtr<TabGroup> tabGroup;
if (aTabOpener && !aForceNoOpener) {
// The new actor will use the same tab group as the opener.
tabGroup = aTabOpener->TabGroup();
} else {
tabGroup = new TabGroup();
}

TabContext newTabContext = aTabOpener ? *aTabOpener : TabContext();
RefPtr<TabChild> newChild = new TabChild(this, tabId,
RefPtr<TabChild> newChild = new TabChild(this, tabId, tabGroup,
newTabContext, aChromeFlags);
if (NS_FAILED(newChild->Init())) {
return NS_ERROR_ABORT;
Expand All @@ -787,23 +798,13 @@ ContentChild::ProvideWindowCommon(TabChild* aTabOpener,
ipcContext->get_PopupIPCTabContext().opener() = aTabOpener;
}

// We need to assign a TabGroup to the PBrowser actor before we send it to the
// parent. Otherwise, the parent could send messages to us before we have a
// proper TabGroup for that actor.
RefPtr<TabGroup> tabGroup;
if (aTabOpener && !aForceNoOpener) {
// The new actor will use the same tab group as the opener.
tabGroup = aTabOpener->TabGroup();
} else {
tabGroup = new TabGroup();
}
nsCOMPtr<nsIEventTarget> target = tabGroup->EventTargetFor(TaskCategory::Other);
SetEventTargetForActor(newChild, target);

Unused << SendPBrowserConstructor(
// We release this ref in DeallocPBrowserChild
RefPtr<TabChild>(newChild).forget().take(),
tabId, *ipcContext, aChromeFlags,
tabId, TabId(0), *ipcContext, aChromeFlags,
GetID(), IsForBrowser());

nsString name(aName);
Expand Down Expand Up @@ -1441,12 +1442,14 @@ ContentChild::DeallocPJavaScriptChild(PJavaScriptChild *aChild)

PBrowserChild*
ContentChild::AllocPBrowserChild(const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
const bool& aIsForBrowser)
{
return nsIContentChild::AllocPBrowserChild(aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand All @@ -1456,6 +1459,7 @@ ContentChild::AllocPBrowserChild(const TabId& aTabId,
bool
ContentChild::SendPBrowserConstructor(PBrowserChild* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
Expand All @@ -1467,6 +1471,7 @@ ContentChild::SendPBrowserConstructor(PBrowserChild* aActor,

return PContentChild::SendPBrowserConstructor(aActor,
aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
Expand All @@ -1476,6 +1481,7 @@ ContentChild::SendPBrowserConstructor(PBrowserChild* aActor,
mozilla::ipc::IPCResult
ContentChild::RecvPBrowserConstructor(PBrowserChild* aActor,
const TabId& aTabId,
const TabId& aSameTabGroupAs,
const IPCTabContext& aContext,
const uint32_t& aChromeFlags,
const ContentParentId& aCpID,
Expand All @@ -1492,8 +1498,13 @@ ContentChild::RecvPBrowserConstructor(PBrowserChild* aActor,
NS_IdleDispatchToCurrentThread(firstIdleTask.forget());
}

return nsIContentChild::RecvPBrowserConstructor(aActor, aTabId, aContext,
aChromeFlags, aCpID, aIsForBrowser);
return nsIContentChild::RecvPBrowserConstructor(aActor,
aTabId,
aSameTabGroupAs,
aContext,
aChromeFlags,
aCpID,
aIsForBrowser);
}

void
Expand Down Expand Up @@ -3170,6 +3181,32 @@ ContentChild::GetConstructedEventTarget(const Message& aMsg)
return nullptr;
}

ActorHandle handle;
TabId tabId, sameTabGroupAs;
PickleIterator iter(aMsg);
if (!IPC::ReadParam(&aMsg, &iter, &handle)) {
return nullptr;
}
aMsg.IgnoreSentinel(&iter);
if (!IPC::ReadParam(&aMsg, &iter, &tabId)) {
return nullptr;
}
aMsg.IgnoreSentinel(&iter);
if (!IPC::ReadParam(&aMsg, &iter, &sameTabGroupAs)) {
return nullptr;
}

// If sameTabGroupAs is non-zero, then the new tab will be in the same
// TabGroup as a previously created tab. Rather than try to find the
// previously created tab (whose constructor message may not even have been
// processed yet, in theory) and look up its event target, we just use the
// default event target. This means that runnables for this tab will not be
// labeled. However, this path is only taken for print preview and view
// source, which are not performance-sensitive.
if (sameTabGroupAs) {
return nullptr;
}

// If the request for a new TabChild is coming from the parent process, then
// there is no opener. Therefore, we create a fresh TabGroup.
RefPtr<TabGroup> tabGroup = new TabGroup();
Expand Down
Loading

0 comments on commit 5c60972

Please sign in to comment.