Backed out d2aa085d7ebd (bug 836605)
authorChris Jones <jones.chris.g@gmail.com>
Wed, 06 Feb 2013 15:49:27 -0800
changeset 130978 11d2d81e066d43428b40c099435d8f8120994dd6
parent 130977 e1092f311f2dbcae8c873e8db94924edc2ad0678
child 130979 bbad510c2ca9e4e4ee76a7dd4a7095b7adf0b0bc
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs836605
milestone21.0a1
backs outd2aa085d7ebdd4d6aa9b18ad42f52ce65477d59d
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out d2aa085d7ebd (bug 836605)
content/base/src/nsFrameLoader.cpp
content/base/src/nsFrameLoader.h
content/html/content/src/nsGenericHTMLFrameElement.cpp
content/html/content/src/nsGenericHTMLFrameElement.h
dom/interfaces/html/Makefile.in
dom/interfaces/html/nsIMozBrowserFrame.idl
dom/ipc/TabContext.cpp
dom/ipc/TabContext.h
--- a/content/base/src/nsFrameLoader.cpp
+++ b/content/base/src/nsFrameLoader.cpp
@@ -1422,53 +1422,76 @@ nsFrameLoader::OwnerIsBrowserOrAppFrame(
   nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
   return browserFrame ? browserFrame->GetReallyIsBrowserOrApp() : false;
 }
 
 bool
 nsFrameLoader::OwnerIsAppFrame()
 {
   nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
-  if (!browserFrame) {
-    return false;
-  }
-  nsCOMPtr<mozIApplication> app;
-  browserFrame->GetOwnApp(getter_AddRefs(app));
-  return !!app;
+  return browserFrame ? browserFrame->GetReallyIsApp() : false;
 }
 
 bool
 nsFrameLoader::OwnerIsBrowserFrame()
 {
   return OwnerIsBrowserOrAppFrame() && !OwnerIsAppFrame();
 }
 
+void
+nsFrameLoader::GetOwnerAppManifestURL(nsAString& aOut)
+{
+  aOut.Truncate();
+  nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
+  if (browserFrame) {
+    browserFrame->GetAppManifestURL(aOut);
+  }
+}
+
 already_AddRefed<mozIApplication>
 nsFrameLoader::GetOwnApp()
 {
-  nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
-  if (!browserFrame) {
+  nsAutoString manifest;
+  GetOwnerAppManifestURL(manifest);
+  if (manifest.IsEmpty()) {
     return nullptr;
   }
 
-  nsCOMPtr<mozIApplication> app;
-  browserFrame->GetOwnApp(getter_AddRefs(app));
+  nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID);
+  NS_ENSURE_TRUE(appsService, nullptr);
+
+  nsCOMPtr<mozIDOMApplication> domApp;
+  appsService->GetAppByManifestURL(manifest, getter_AddRefs(domApp));
+
+  nsCOMPtr<mozIApplication> app = do_QueryInterface(domApp);
+  MOZ_ASSERT_IF(domApp, app);
   return app.forget();
 }
 
 already_AddRefed<mozIApplication>
 nsFrameLoader::GetContainingApp()
 {
-  nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
-  if (!browserFrame) {
+  // See if our owner content's principal has an associated app.
+  uint32_t appId = mOwnerContent->NodePrincipal()->GetAppId();
+  MOZ_ASSERT(appId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
+
+  if (appId == nsIScriptSecurityManager::NO_APP_ID ||
+      appId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
     return nullptr;
   }
 
-  nsCOMPtr<mozIApplication> app;
-  browserFrame->GetContainingApp(getter_AddRefs(app));
+  nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID);
+  NS_ENSURE_TRUE(appsService, nullptr);
+
+  nsCOMPtr<mozIDOMApplication> domApp;
+  appsService->GetAppByLocalId(appId, getter_AddRefs(domApp));
+  MOZ_ASSERT(domApp);
+
+  nsCOMPtr<mozIApplication> app = do_QueryInterface(domApp);
+  MOZ_ASSERT_IF(domApp, app);
   return app.forget();
 }
 
 bool
 nsFrameLoader::ShouldUseRemoteProcess()
 {
   if (PR_GetEnv("MOZ_DISABLE_OOP_TABS") ||
       Preferences::GetBool("dom.ipc.tabs.disabled", false)) {
@@ -2042,17 +2065,17 @@ nsFrameLoader::TryRemoteBrowser()
     nsCOMPtr<nsIDOMElement> element = do_QueryInterface(mOwnerContent);
     mRemoteBrowser->SetOwnerElement(element);
 
     // If we're an app, send the frame element's mozapptype down to the child
     // process.  This ends up in TabChild::GetAppType().
     if (ownApp) {
       nsAutoString appType;
       mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::mozapptype, appType);
-      unused << mRemoteBrowser->SendSetAppType(appType);
+      mRemoteBrowser->SendSetAppType(appType);
     }
 
     nsCOMPtr<nsIDocShellTreeItem> rootItem;
     parentAsItem->GetRootTreeItem(getter_AddRefs(rootItem));
     nsCOMPtr<nsIDOMWindow> rootWin = do_GetInterface(rootItem);
     nsCOMPtr<nsIDOMChromeWindow> rootChromeWin = do_QueryInterface(rootWin);
     NS_ABORT_IF_FALSE(rootChromeWin, "How did we not get a chrome window here?");
 
--- a/content/base/src/nsFrameLoader.h
+++ b/content/base/src/nsFrameLoader.h
@@ -326,16 +326,22 @@ private:
   bool OwnerIsAppFrame();
 
   /**
    * Is this a frame loader for a bona fide <iframe mozbrowser>?
    */
   bool OwnerIsBrowserFrame();
 
   /**
+   * Get our owning element's app manifest URL, or return the empty string if
+   * our owning element doesn't have an app manifest URL.
+   */
+  void GetOwnerAppManifestURL(nsAString& aOut);
+
+  /**
    * Get the app for our frame.  This is the app whose manifest is returned by
    * GetOwnerAppManifestURL.
    */
   already_AddRefed<mozIApplication> GetOwnApp();
 
   /**
    * Get the app which contains this frame.  This is the app associated with
    * the frame element's principal.
--- a/content/html/content/src/nsGenericHTMLFrameElement.cpp
+++ b/content/html/content/src/nsGenericHTMLFrameElement.cpp
@@ -8,17 +8,16 @@
 #include "nsGenericHTMLFrameElement.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsContentUtils.h"
 #include "mozilla/Preferences.h"
 #include "nsIAppsService.h"
 #include "nsServiceManagerUtils.h"
 #include "nsIDOMApplicationRegistry.h"
 #include "nsIPermissionManager.h"
-#include "nsIScriptSecurityManager.h"
 #include "sampler.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsGenericHTMLFrameElement,
                                                   nsGenericHTMLElement)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFrameLoader)
@@ -307,103 +306,66 @@ nsGenericHTMLFrameElement::GetReallyIsBr
 
   uint32_t permission = nsIPermissionManager::DENY_ACTION;
   nsresult rv = permMgr->TestPermissionFromPrincipal(principal, "browser", &permission);
   NS_ENSURE_SUCCESS(rv, NS_OK);
   *aOut = permission == nsIPermissionManager::ALLOW_ACTION;
   return NS_OK;
 }
 
-bool
-nsGenericHTMLFrameElement::MayBeAppFrame()
+/* [infallible] */ NS_IMETHODIMP
+nsGenericHTMLFrameElement::GetReallyIsApp(bool *aOut)
 {
+  nsAutoString manifestURL;
+  GetAppManifestURL(manifestURL);
+
+  *aOut = !manifestURL.IsEmpty();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsGenericHTMLFrameElement::GetAppManifestURL(nsAString& aOut)
+{
+  aOut.Truncate();
+
   // At the moment, you can't be an app without being a browser.
   if (!nsIMozBrowserFrame::GetReallyIsBrowserOrApp()) {
-    return false;
+    return NS_OK;
   }
 
   // Check permission.
   nsIPrincipal *principal = NodePrincipal();
   nsCOMPtr<nsIPermissionManager> permMgr =
     do_GetService(NS_PERMISSIONMANAGER_CONTRACTID);
-  NS_ENSURE_TRUE(permMgr, false);
+  NS_ENSURE_TRUE(permMgr, NS_OK);
 
   uint32_t permission = nsIPermissionManager::DENY_ACTION;
   nsresult rv = permMgr->TestPermissionFromPrincipal(principal,
                                                      "embed-apps",
                                                      &permission);
-  NS_ENSURE_SUCCESS(rv, false);
-
-  return (permission == nsIPermissionManager::ALLOW_ACTION);
-}
-
-NS_IMETHODIMP
-nsGenericHTMLFrameElement::GetOwnApp(mozIApplication** aApp)
-{
-  *aApp = nullptr;
-
-  if (!MayBeAppFrame()) {
-    return NS_OK;
-  }
-
-  if (mApp) {
-    nsCOMPtr<mozIApplication>(mApp).forget(aApp);
+  NS_ENSURE_SUCCESS(rv, NS_OK);
+  if (permission != nsIPermissionManager::ALLOW_ACTION) {
     return NS_OK;
   }
 
   nsAutoString manifestURL;
   GetAttr(kNameSpaceID_None, nsGkAtoms::mozapp, manifestURL);
   if (manifestURL.IsEmpty()) {
     return NS_OK;
   }
 
   nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID);
   NS_ENSURE_TRUE(appsService, NS_OK);
 
   nsCOMPtr<mozIDOMApplication> app;
   appsService->GetAppByManifestURL(manifestURL, getter_AddRefs(app));
-  mApp = do_QueryInterface(app);
-  nsCOMPtr<mozIApplication>(mApp).forget(aApp);
-
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsGenericHTMLFrameElement::GetContainingApp(mozIApplication** aApp)
-{
-  *aApp = nullptr;
-
-  if (!MayBeAppFrame()) {
-    return NS_OK;
-  }
-
-  if (mContainingApp) {
-    nsCOMPtr<mozIApplication>(mContainingApp).forget(aApp);
-    return NS_OK;
+  if (app) {
+    aOut.Assign(manifestURL);
   }
 
-  uint32_t appId = NodePrincipal()->GetAppId();
-  MOZ_ASSERT(appId != nsIScriptSecurityManager::UNKNOWN_APP_ID);
-
-  if (appId == nsIScriptSecurityManager::NO_APP_ID ||
-      appId == nsIScriptSecurityManager::UNKNOWN_APP_ID) {
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIAppsService> appsService = do_GetService(APPS_SERVICE_CONTRACTID);
-  NS_ENSURE_TRUE(appsService, NS_OK);
-
-  nsCOMPtr<mozIDOMApplication> domApp;
-  appsService->GetAppByLocalId(appId, getter_AddRefs(domApp));
-  MOZ_ASSERT(domApp);
-
-  mContainingApp = do_QueryInterface(domApp);
-  MOZ_ASSERT_IF(domApp, mContainingApp);
-
-  nsCOMPtr<mozIApplication>(mContainingApp).forget(aApp);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsGenericHTMLFrameElement::DisallowCreateFrameLoader()
 {
   MOZ_ASSERT(!mFrameLoader);
   MOZ_ASSERT(!mFrameLoaderCreationDisallowed);
--- a/content/html/content/src/nsGenericHTMLFrameElement.h
+++ b/content/html/content/src/nsGenericHTMLFrameElement.h
@@ -85,31 +85,17 @@ protected:
 
   // This doesn't really ensure a frame loade in all cases, only when
   // it makes sense.
   nsresult EnsureFrameLoader();
   nsresult LoadSrc();
   nsresult GetContentDocument(nsIDOMDocument** aContentDocument);
   nsresult GetContentWindow(nsIDOMWindow** aContentWindow);
 
-  // Return true iff this frame may be an app frame.  This is the case
-  // if we're an app or browser frame and our containing app has the
-  // "embed-apps" permission.
-  bool MayBeAppFrame();
-
   nsRefPtr<nsFrameLoader> mFrameLoader;
-  // These elements are a cache to avoid calling out to the app
-  // service to look up app IDs.  The getters that access these
-  // attributes are on the critical startup path, and constructing
-  // these objects (which are implemented in JS) is expensive enough
-  // to show up on profiles.  The app service can't cache the objects
-  // itself because the returned objects are mutable.  However, our
-  // use of them is immutable.
-  nsCOMPtr<mozIApplication> mApp;
-  nsCOMPtr<mozIApplication> mContainingApp;
 
   // True when the element is created by the parser
   // using NS_FROM_PARSER_NETWORK flag.
   // If the element is modified, it may lose the flag.
   bool                    mNetworkCreated;
 
   bool                    mBrowserFrameListenersRegistered;
   bool                    mFrameLoaderCreationDisallowed;
--- a/dom/interfaces/html/Makefile.in
+++ b/dom/interfaces/html/Makefile.in
@@ -91,12 +91,11 @@ XPIDLSRCS = 					\
 	nsIDOMHTMLCanvasElement.idl		\
 	nsIDOMHTMLUnknownElement.idl \
 	nsIMozBrowserFrame.idl \
 	$(NULL)
 
 include $(topsrcdir)/config/rules.mk
 
 XPIDL_FLAGS += \
-  -I$(topsrcdir)/dom/interfaces/apps \
   -I$(topsrcdir)/dom/interfaces/base \
   -I$(topsrcdir)/dom/interfaces/core \
   $(NULL)
--- a/dom/interfaces/html/nsIMozBrowserFrame.idl
+++ b/dom/interfaces/html/nsIMozBrowserFrame.idl
@@ -1,45 +1,47 @@
 /* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim:set tw=80 expandtab softtabstop=2 ts=2 sw=2: */
 
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#include "mozIApplication.idl"
 #include "nsIDOMMozBrowserFrame.idl"
 
-interface mozIMozApplication;
 interface nsITabParent;
 
-[scriptable, builtinclass, uuid(a2373fa5-a090-4192-ab28-6df793a631c1)]
+[scriptable, builtinclass, uuid(929AED00-3E15-49B7-8CA2-75003715B7E7)]
 interface nsIMozBrowserFrame : nsIDOMMozBrowserFrame
 {
   /**
    * Gets whether this frame really is a browser or app frame.
    *
    * In order to really be a browser frame, this frame's
    * nsIDOMMozBrowserFrame::mozbrowser attribute must be true, and the frame
    * may have to pass various security checks.
    */
   [infallible] readonly attribute boolean reallyIsBrowserOrApp;
 
   /**
-   * Get this frame's app, if the frame really is an app frame.
-   * Otherwise, return null.
+   * Gets whether this frame really is an app frame.
+   *
+   * In order to really be an app frame, this frame must really be a browser
+   * frame (this requirement will go away eventually), and the frame's mozapp
+   * attribute must point to the manifest of a valid app.
    */
-  readonly attribute mozIApplication ownApp;
+  [infallible] readonly attribute boolean reallyIsApp;
 
   /**
-   * Get the app that contains this frame, if the frame really is an
-   * app or browser frame.  This is the app associated with the frame
-   * element's principal.  Otherwise, return null.
+   * Gets this frame's app manifest URL, if the frame really is an app frame.
+   * Otherwise, returns the empty string.
+   *
+   * This method is guaranteed not to fail.
    */
-  readonly attribute mozIApplication containingApp;
+  readonly attribute AString appManifestURL;
 
   /**
    * Normally, a frame tries to create its frame loader when its src is
    * modified, or its contentWindow is accessed.
    *
    * disallowCreateFrameLoader prevents the frame element from creating its
    * frame loader (in the same way that not being inside a document prevents the
    * creation of a frame loader).  allowCreateFrameLoader lifts this restriction.
--- a/dom/ipc/TabContext.cpp
+++ b/dom/ipc/TabContext.cpp
@@ -119,19 +119,16 @@ uint32_t
 TabContext::OwnAppId() const
 {
   return mOwnAppId;
 }
 
 already_AddRefed<mozIApplication>
 TabContext::GetOwnApp() const
 {
-  if (mOwnApp) {
-    return nsCOMPtr<mozIApplication>(mOwnApp).forget();
-  }
   return GetAppForId(OwnAppId());
 }
 
 bool
 TabContext::HasOwnApp() const
 {
   return mOwnAppId != nsIScriptSecurityManager::NO_APP_ID;
 }
@@ -164,19 +161,16 @@ TabContext::AppOwnerAppId() const
     return mContainingAppId;
   }
   return nsIScriptSecurityManager::NO_APP_ID;
 }
 
 already_AddRefed<mozIApplication>
 TabContext::GetAppOwnerApp() const
 {
-  if (mContainingApp) {
-    return nsCOMPtr<mozIApplication>(mContainingApp).forget();
-  }
   return GetAppForId(AppOwnerAppId());
 }
 
 bool
 TabContext::HasAppOwnerApp() const
 {
   return AppOwnerAppId() != nsIScriptSecurityManager::NO_APP_ID;
 }
@@ -194,22 +188,16 @@ TabContext::OwnOrContainingAppId() const
   }
 
   return mContainingAppId;
 }
 
 already_AddRefed<mozIApplication>
 TabContext::GetOwnOrContainingApp() const
 {
-  if (mOwnApp) {
-    return nsCOMPtr<mozIApplication>(mOwnApp).forget();
-  }
-  if (mContainingApp) {
-    return nsCOMPtr<mozIApplication>(mContainingApp).forget();
-  }
   return GetAppForId(OwnOrContainingAppId());
 }
 
 bool
 TabContext::HasOwnOrContainingApp() const
 {
   return OwnOrContainingAppId() != nsIScriptSecurityManager::NO_APP_ID;
 }
@@ -257,19 +245,17 @@ TabContext::SetTabContextForAppFrame(moz
   if (aAppFrameOwnerApp) {
     nsresult rv = aOwnApp->GetLocalId(&containingAppId);
     NS_ENSURE_SUCCESS(rv, false);
   }
 
   mInitialized = true;
   mIsBrowser = false;
   mOwnAppId = ownAppId;
-  mOwnApp = aOwnApp;
   mContainingAppId = containingAppId;
-  mContainingApp = aAppFrameOwnerApp;
   mScrollingBehavior = aRequestedBehavior;
   return true;
 }
 
 bool
 TabContext::SetTabContextForBrowserFrame(mozIApplication* aBrowserFrameOwnerApp,
                                          ScrollingBehavior aRequestedBehavior)
 {
--- a/dom/ipc/TabContext.h
+++ b/dom/ipc/TabContext.h
@@ -168,36 +168,24 @@ private:
    */
   bool mInitialized;
 
   /**
    * This TabContext's own app id.  If this is something other than NO_APP_ID,
    * then this TabContext corresponds to an app, and mIsBrowser must be false.
    */
   uint32_t mOwnAppId;
-  /**
-   * This is a cache of the app object that would be returned from the
-   * apps service by looking up mOwnAppId.  This lookup and object
-   * creation can be expensive and is on critical startup paths.
-   *
-   * The object returned from the apps service is mutable, but our use
-   * of that object must be immutable.  This object may be cached by
-   * other clients that treat the object as immutable.
-   */
-  nsCOMPtr<mozIApplication> mOwnApp;
 
   /**
    * The id of the app which contains this TabContext's frame.  If mIsBrowser,
    * this corresponds to the ID of the app which contains the browser frame;
    * otherwise, this correspodns to the ID of the app which contains the app
    * frame.
    */
   uint32_t mContainingAppId;
-  /** See comment above describing mOwnApp. */
-  nsCOMPtr<mozIApplication> mContainingApp;
 
   /**
    * The requested scrolling behavior for this frame.
    */
   ScrollingBehavior mScrollingBehavior;
 
   /**
    * Does this TabContext correspond to a browser element?