Bug 836605: Cache mozIApplication wherever possible on critical startup path. r=jlebar

content/base/src/nsFrameLoader.cpp

@@ -1427,7 +1427,12 @@ bool
 nsFrameLoader::OwnerIsAppFrame()
 {
   nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
-  return browserFrame ? browserFrame->GetReallyIsApp() : false;
+  if (!browserFrame) {
+    return false;
+  }
+  nsCOMPtr<mozIApplication> app;
+  browserFrame->GetOwnApp(getter_AddRefs(app));
+  return !!app;
 }
 
 bool
@@ -1436,57 +1441,29 @@ 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()
 {
-  nsAutoString manifest;
-  GetOwnerAppManifestURL(manifest);
-  if (manifest.IsEmpty()) {
+  nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
+  if (!browserFrame) {
     return nullptr;
   }
 
-  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);
+  nsCOMPtr<mozIApplication> app;
+  browserFrame->GetOwnApp(getter_AddRefs(app));
   return app.forget();
 }
 
 already_AddRefed<mozIApplication>
 nsFrameLoader::GetContainingApp()
 {
-  // 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) {
+  nsCOMPtr<nsIMozBrowserFrame> browserFrame = do_QueryInterface(mOwnerContent);
+  if (!browserFrame) {
     return nullptr;
   }
 
-  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);
+  nsCOMPtr<mozIApplication> app;
+  browserFrame->GetContainingApp(getter_AddRefs(app));
   return app.forget();
 }
 
@@ -2070,7 +2047,7 @@ nsFrameLoader::TryRemoteBrowser()
     if (ownApp) {
       nsAutoString appType;
       mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::mozapptype, appType);
-      mRemoteBrowser->SendSetAppType(appType);
+      unused << mRemoteBrowser->SendSetAppType(appType);
     }
 
     nsCOMPtr<nsIDocShellTreeItem> rootItem;

content/base/src/nsFrameLoader.h

@@ -330,12 +330,6 @@ private:
    */
   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.

content/html/content/src/nsGenericHTMLFrameElement.cpp

@@ -13,6 +13,7 @@
 #include "nsServiceManagerUtils.h"
 #include "nsIDOMApplicationRegistry.h"
 #include "nsIPermissionManager.h"
+#include "nsIScriptSecurityManager.h"
 #include "sampler.h"
 
 using namespace mozilla;
@@ -311,38 +312,40 @@ nsGenericHTMLFrameElement::GetReallyIsBrowserOrApp(bool *aOut)
   return NS_OK;
 }
 
-/* [infallible] */ NS_IMETHODIMP
-nsGenericHTMLFrameElement::GetReallyIsApp(bool *aOut)
+bool
+nsGenericHTMLFrameElement::MayBeAppFrame()
 {
-  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 NS_OK;
+    return false;
   }
 
   // Check permission.
   nsIPrincipal *principal = NodePrincipal();
   nsCOMPtr<nsIPermissionManager> permMgr =
     do_GetService(NS_PERMISSIONMANAGER_CONTRACTID);
-  NS_ENSURE_TRUE(permMgr, NS_OK);
+  NS_ENSURE_TRUE(permMgr, false);
 
   uint32_t permission = nsIPermissionManager::DENY_ACTION;
   nsresult rv = permMgr->TestPermissionFromPrincipal(principal,
                                                      "embed-apps",
                                                      &permission);
-  NS_ENSURE_SUCCESS(rv, NS_OK);
-  if (permission != nsIPermissionManager::ALLOW_ACTION) {
+  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);
     return NS_OK;
   }
 
@@ -357,10 +360,45 @@ nsGenericHTMLFrameElement::GetAppManifestURL(nsAString& aOut)
 
   nsCOMPtr<mozIDOMApplication> app;
   appsService->GetAppByManifestURL(manifestURL, getter_AddRefs(app));
-  if (app) {
-    aOut.Assign(manifestURL);
+  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;
+  }
+
+  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;
 }
 

content/html/content/src/nsGenericHTMLFrameElement.h

@@ -90,7 +90,21 @@ protected:
   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.

dom/interfaces/html/Makefile.in

@@ -96,6 +96,7 @@ XPIDLSRCS = 					\
 include $(topsrcdir)/config/rules.mk
 
 XPIDL_FLAGS += \
+  -I$(topsrcdir)/dom/interfaces/apps \
   -I$(topsrcdir)/dom/interfaces/base \
   -I$(topsrcdir)/dom/interfaces/core \
   $(NULL)

dom/interfaces/html/nsIMozBrowserFrame.idl

@@ -5,11 +5,13 @@
  * 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(929AED00-3E15-49B7-8CA2-75003715B7E7)]
+[scriptable, builtinclass, uuid(a2373fa5-a090-4192-ab28-6df793a631c1)]
 interface nsIMozBrowserFrame : nsIDOMMozBrowserFrame
 {
   /**
@@ -22,21 +24,17 @@ interface nsIMozBrowserFrame : nsIDOMMozBrowserFrame
   [infallible] readonly attribute boolean reallyIsBrowserOrApp;
 
   /**
-   * 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.
+   * Get this frame's app, if the frame really is an app frame.
+   * Otherwise, return null.
    */
-  [infallible] readonly attribute boolean reallyIsApp;
+  readonly attribute mozIApplication ownApp;
 
   /**
-   * 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.
+   * 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.
    */
-  readonly attribute AString appManifestURL;
+  readonly attribute mozIApplication containingApp;
 
   /**
    * Normally, a frame tries to create its frame loader when its src is

dom/ipc/TabContext.cpp

@@ -124,6 +124,9 @@ TabContext::OwnAppId() const
 already_AddRefed<mozIApplication>
 TabContext::GetOwnApp() const
 {
+  if (mOwnApp) {
+    return nsCOMPtr<mozIApplication>(mOwnApp).forget();
+  }
   return GetAppForId(OwnAppId());
 }
 
@@ -166,6 +169,9 @@ TabContext::AppOwnerAppId() const
 already_AddRefed<mozIApplication>
 TabContext::GetAppOwnerApp() const
 {
+  if (mContainingApp) {
+    return nsCOMPtr<mozIApplication>(mContainingApp).forget();
+  }
   return GetAppForId(AppOwnerAppId());
 }
 
@@ -193,6 +199,12 @@ TabContext::OwnOrContainingAppId() const
 already_AddRefed<mozIApplication>
 TabContext::GetOwnOrContainingApp() const
 {
+  if (mOwnApp) {
+    return nsCOMPtr<mozIApplication>(mOwnApp).forget();
+  }
+  if (mContainingApp) {
+    return nsCOMPtr<mozIApplication>(mContainingApp).forget();
+  }
   return GetAppForId(OwnOrContainingAppId());
 }
 
@@ -250,7 +262,9 @@ TabContext::SetTabContextForAppFrame(mozIApplication* aOwnApp, mozIApplication*
   mInitialized = true;
   mIsBrowser = false;
   mOwnAppId = ownAppId;
+  mOwnApp = aOwnApp;
   mContainingAppId = containingAppId;
+  mContainingApp = aAppFrameOwnerApp;
   mScrollingBehavior = aRequestedBehavior;
   return true;
 }

dom/ipc/TabContext.h

@@ -173,6 +173,16 @@ private:
    * 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,
@@ -181,6 +191,8 @@ private:
    * frame.
    */
   uint32_t mContainingAppId;
+  /** See comment above describing mOwnApp. */
+  nsCOMPtr<mozIApplication> mContainingApp;
 
   /**
    * The requested scrolling behavior for this frame.