From d870f5de82b40dfaaef1e728625057c86b902683 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 8 Aug 2012 19:58:06 -0700 Subject: [PATCH] Bug 776649, part 2: Refactor content-process/browser creation to use mozIApplication for passing app info. r=jlebar --- content/base/src/nsFrameLoader.cpp | 32 +++++---- dom/ipc/ContentChild.cpp | 8 ++- dom/ipc/ContentChild.h | 2 +- dom/ipc/ContentParent.cpp | 104 +++++++++++++++++++++-------- dom/ipc/ContentParent.h | 26 ++++---- dom/ipc/PContent.ipdl | 18 ++++- dom/ipc/TabChild.cpp | 11 +-- dom/ipc/TabChild.h | 2 + dom/ipc/TabParent.cpp | 8 ++- dom/ipc/TabParent.h | 8 ++- 10 files changed, 148 insertions(+), 71 deletions(-) diff --git a/content/base/src/nsFrameLoader.cpp b/content/base/src/nsFrameLoader.cpp index f87f0678e2ed..f52f27d4099e 100644 --- a/content/base/src/nsFrameLoader.cpp +++ b/content/base/src/nsFrameLoader.cpp @@ -13,6 +13,7 @@ #include "prenv.h" +#include "mozIApplication.h" #include "nsIDOMHTMLIFrameElement.h" #include "nsIDOMHTMLFrameElement.h" #include "nsIDOMMozBrowserFrame.h" @@ -31,6 +32,7 @@ #include "nsIDocShellTreeNode.h" #include "nsIDocShellTreeOwner.h" #include "nsIDocShellLoadInfo.h" +#include "nsIDOMApplicationRegistry.h" #include "nsIBaseWindow.h" #include "nsContentUtils.h" #include "nsIXPConnect.h" @@ -1973,9 +1975,8 @@ nsFrameLoader::TryRemoteBrowser() return false; } - PRUint32 appId = 0; bool isBrowserElement = false; - + nsCOMPtr app; if (OwnerIsBrowserFrame()) { isBrowserElement = true; @@ -1989,24 +1990,21 @@ nsFrameLoader::TryRemoteBrowser() return false; } - appsService->GetAppLocalIdByManifestURL(manifest, &appId); - - // If the frame is actually an app, we should not mark it as a browser. - if (appId != nsIScriptSecurityManager::NO_APP_ID) { + nsCOMPtr domApp; + appsService->GetAppByManifestURL(manifest, getter_AddRefs(domApp)); + // If the frame is actually an app, we should not mark it as a + // browser. This is to identify the data store: since s + // and s-within-s have different stores, we want + // to ensure the uses its store, not the one for its + // s. + app = do_QueryInterface(domApp); + if (app) { isBrowserElement = false; } } } - // If our owner has no app manifest URL, then this is equivalent to - // ContentParent::GetNewOrUsed(). - nsAutoString appManifest; - GetOwnerAppManifestURL(appManifest); - ContentParent* parent = ContentParent::GetForApp(appManifest); - - NS_ASSERTION(parent->IsAlive(), "Process parent should be alive; something is very wrong!"); - mRemoteBrowser = parent->CreateTab(chromeFlags, isBrowserElement, appId); - if (mRemoteBrowser) { + if ((mRemoteBrowser = ContentParent::CreateBrowser(app, isBrowserElement))) { nsCOMPtr element = do_QueryInterface(mOwnerContent); mRemoteBrowser->SetOwnerElement(element); @@ -2019,8 +2017,8 @@ nsFrameLoader::TryRemoteBrowser() nsCOMPtr browserDOMWin; rootChromeWin->GetBrowserDOMWindow(getter_AddRefs(browserDOMWin)); mRemoteBrowser->SetBrowserDOMWindow(browserDOMWin); - - mChildHost = parent; + + mChildHost = static_cast(mRemoteBrowser->Manager()); } return true; } diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index db17054a7cf2..f4fdb9e23d06 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -39,6 +39,7 @@ #include "nsIObserverService.h" #include "nsTObserverArray.h" #include "nsIObserver.h" +#include "nsIScriptSecurityManager.h" #include "nsServiceManagerUtils.h" #include "nsXULAppAPI.h" #include "nsWeakReference.h" @@ -402,10 +403,11 @@ ContentChild::AllocPCompositor(mozilla::ipc::Transport* aTransport, PBrowserChild* ContentChild::AllocPBrowser(const PRUint32& aChromeFlags, - const bool& aIsBrowserElement, - const PRUint32& aAppId) + const bool& aIsBrowserElement, const AppId& aApp) { - nsRefPtr iframe = new TabChild(aChromeFlags, aIsBrowserElement, aAppId); + PRUint32 appId = aApp.get_uint32_t(); + nsRefPtr iframe = new TabChild(aChromeFlags, aIsBrowserElement, + appId); return NS_SUCCEEDED(iframe->Init()) ? iframe.forget().get() : NULL; } diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 879fe81112ef..d11d93dceb66 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -68,7 +68,7 @@ public: virtual PBrowserChild* AllocPBrowser(const PRUint32& aChromeFlags, const bool& aIsBrowserElement, - const PRUint32& aAppId); + const AppId& aAppId); virtual bool DeallocPBrowser(PBrowserChild*); virtual PDeviceStorageRequestChild* AllocPDeviceStorageRequest(const DeviceStorageParams&); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 94758f22e5fd..d3107d3a276d 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -20,6 +20,7 @@ #include "IDBFactory.h" #include "IndexedDBParent.h" #include "IndexedDatabaseManager.h" +#include "mozIApplication.h" #include "mozilla/Preferences.h" #include "mozilla/Preferences.h" #include "mozilla/Services.h" @@ -49,8 +50,10 @@ #include "nsFrameMessageManager.h" #include "nsHashPropertyBag.h" #include "nsIAlertsService.h" +#include "nsIAppsService.h" #include "nsIClipboard.h" #include "nsIConsoleService.h" +#include "nsIDOMApplicationRegistry.h" #include "nsIDOMGeoGeolocation.h" #include "nsIDOMWindow.h" #include "nsIFilePicker.h" @@ -59,6 +62,7 @@ #include "nsIPresShell.h" #include "nsIRemoteBlob.h" #include "nsIScriptError.h" +#include "nsIScriptSecurityManager.h" #include "nsISupportsPrimitives.h" #include "nsIWindowWatcher.h" #include "nsMemoryReporterManager.h" @@ -149,7 +153,7 @@ nsTArray* ContentParent::gPrivateContent; // The first content child has ID 1, so the chrome process can have ID 0. static PRUint64 gContentChildID = 1; -ContentParent* +/*static*/ ContentParent* ContentParent::GetNewOrUsed() { if (!gNonAppContentParents) @@ -165,7 +169,7 @@ ContentParent::GetNewOrUsed() NS_ASSERTION(p->IsAlive(), "Non-alive contentparent in gNonAppContentParents?"); return p; } - + nsRefPtr p = new ContentParent(/* appManifestURL = */ EmptyString()); p->Init(); @@ -173,11 +177,20 @@ ContentParent::GetNewOrUsed() return p; } -ContentParent* -ContentParent::GetForApp(const nsAString& aAppManifestURL) +/*static*/ TabParent* +ContentParent::CreateBrowser(mozIApplication* aApp, bool aIsBrowserElement) { - if (aAppManifestURL.IsEmpty()) { - return GetNewOrUsed(); + if (!aApp) { + if (ContentParent* cp = GetNewOrUsed()) { + nsRefPtr tp(new TabParent(aApp, aIsBrowserElement)); + return static_cast( + cp->SendPBrowserConstructor( + // DeallocPBrowserParent() releases the ref we take here + tp.forget().get(), + /*chromeFlags*/0, + aIsBrowserElement, nsIScriptSecurityManager::NO_APP_ID)); + } + return nullptr; } if (!gAppContentParents) { @@ -187,14 +200,39 @@ ContentParent::GetForApp(const nsAString& aAppManifestURL) } // Each app gets its own ContentParent instance. - ContentParent* p = gAppContentParents->Get(aAppManifestURL); - if (!p) { - p = new ContentParent(aAppManifestURL); - p->Init(); - gAppContentParents->Put(aAppManifestURL, p); + nsAutoString manifestURL; + if (NS_FAILED(aApp->GetManifestURL(manifestURL))) { + NS_ERROR("Failed to get manifest URL"); + return nullptr; } - return p; + nsCOMPtr appsService = do_GetService(APPS_SERVICE_CONTRACTID); + if (!appsService) { + NS_ERROR("Failed to get apps service"); + return nullptr; + } + + // Send the local app ID to the new TabChild so it knows what app + // it is. + PRUint32 appId; + if (NS_FAILED(appsService->GetAppLocalIdByManifestURL(manifestURL, &appId))) { + NS_ERROR("Failed to get local app ID"); + return nullptr; + } + + ContentParent* p = gAppContentParents->Get(manifestURL); + if (!p) { + p = new ContentParent(manifestURL); + p->Init(); + gAppContentParents->Put(manifestURL, p); + } + + nsRefPtr tp(new TabParent(aApp, aIsBrowserElement)); + return static_cast( + // DeallocPBrowserParent() releases the ref we take here + p->SendPBrowserConstructor(tp.forget().get(), + /*chromeFlags*/0, + aIsBrowserElement, appId)); } static PLDHashOperator @@ -455,12 +493,6 @@ ContentParent::ActorDestroy(ActorDestroyReason why) NS_DispatchToCurrentThread(new DelayedDeleteContentParentTask(this)); } -TabParent* -ContentParent::CreateTab(PRUint32 aChromeFlags, bool aIsBrowserElement, PRUint32 aAppId) -{ - return static_cast(SendPBrowserConstructor(aChromeFlags, aIsBrowserElement, aAppId)); -} - void ContentParent::NotifyTabDestroyed(PBrowserParent* aTab) { @@ -862,22 +894,40 @@ ContentParent::AllocPCompositor(mozilla::ipc::Transport* aTransport, PBrowserParent* ContentParent::AllocPBrowser(const PRUint32& aChromeFlags, - const bool& aIsBrowserElement, - const PRUint32& aAppId) + const bool& aIsBrowserElement, const AppId& aApp) { - TabParent* parent = new TabParent(); - if (parent){ + // We only use this Alloc() method when the content processes asks + // us to open a window. In that case, we're expecting to see the + // opening PBrowser as its app descriptor, and we can trust the data + // associated with that PBrowser since it's fully owned by this + // process. + if (AppId::TPBrowserParent != aApp.type()) { + NS_ERROR("Content process attempting to forge app ID"); + return nullptr; + } + TabParent* opener = static_cast(aApp.get_PBrowserParent()); + + // Popup windows of isBrowser frames are isBrowser if the parent + // isBrowser. Allocating a !isBrowser frame with same app ID + // would allow the content to access data it's not supposed to. + if (opener && opener->IsBrowserElement() && !aIsBrowserElement) { + NS_ERROR("Content process attempting to escalate data access privileges"); + return nullptr; + } + + TabParent* parent = new TabParent(opener ? opener->GetApp() : nullptr, + aIsBrowserElement); + // We release this ref in DeallocPBrowser() NS_ADDREF(parent); - } - return parent; + return parent; } bool ContentParent::DeallocPBrowser(PBrowserParent* frame) { - TabParent* parent = static_cast(frame); - NS_RELEASE(parent); - return true; + TabParent* parent = static_cast(frame); + NS_RELEASE(parent); + return true; } PDeviceStorageRequestParent* diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index 96206fcbfec3..e78eac15d407 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -26,6 +26,7 @@ #include "nsInterfaceHashtable.h" #include "nsHashKeys.h" +class mozIApplication; class nsFrameMessageManager; class nsIDOMBlob; @@ -60,13 +61,16 @@ public: static ContentParent* GetNewOrUsed(); /** - * Get or create a content process for the given app. A given app - * (identified by its manifest URL) gets one process all to itself. + * Get or create a content process for the given app descriptor, + * which may be null. This function will assign processes to app + * or non-app browsers by internal heuristics. * - * If the given manifest is the empty string, then this method is equivalent - * to GetNewOrUsed(). + * Currently apps are given their own process, and browser tabs + * share processes. */ - static ContentParent* GetForApp(const nsAString& aManifestURL); + static TabParent* CreateBrowser(mozIApplication* aApp, + bool aIsBrowserFrame); + static void GetAll(nsTArray& aArray); NS_DECL_ISUPPORTS @@ -74,14 +78,6 @@ public: NS_DECL_NSITHREADOBSERVER NS_DECL_NSIDOMGEOPOSITIONCALLBACK - /** - * Create a new tab. - * - * |aIsBrowserElement| indicates whether this tab is part of an - *