Closed Bug 784805 Opened 12 years ago Closed 12 years ago

plugin-container should update name that shows up in ps

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: dhylands, Assigned: dhylands)

Details

Attachments

(2 files)

Adds a name field to mozIApplication
2.71 KB, patch
fabrice
: review+
Details | Diff | Splinter Review
Sets the process name to match the app name
7.78 KB, patch
cjones
: review+
Details | Diff | Splinter Review 
We currently launch an instance of plugin-container for each OOP app.

We should have the plugin-container change its name (via prctl PR_SET_NAME) to reflect the app which is currently being run.
Let's make an ipc/glue/ProcessUtils.h with a SetThisProcessName() interface or something like that.  The implementation can go in ProcessUtils_linux.

We'll want to initialize the name from here http://mxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild.cpp#928 .  If |app && !browser|, let's use the name "(App)".  Otherwise, "Browser".

To set the process name "for real", you'll want to hook in at http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp#641 .  If !browser, use mAppId to look up the app name.

I think you want to use http://mxr.mozilla.org/mozilla-central/source/dom/interfaces/apps/nsIAppsService.idl#31 to do that, but fabrice could give better advice.
This sounds like a great idea to me.

Dave, are you planning to take this?
I was thinking about it. It's not a blocker, but it would be still be useful. So I've been looking at it in the background.
Assignee: nobody → dhylands
I tried to use the GetManifest method (of mozIDOMApplication) and the manifest I get back is always undefined (i.e. manifest.isUndefined() returns true).

I tried from both the TabChild::RecvShow and from the ContentParent::CreateBrowser

In both of the locations where GetManifest returns an undefined manifest I was able to call GetManifestURL and get back the correct string.
cc mounir re comment 4; I'm not sure how this is supposed to work.
I believe this is because _cloneAppObject doesn't set the manifest. AFAIUI, simply adding |manifest: aApp.manifest,| should fix it.
Adds a name field to moziApplication and initializes it from the manifest
Attachment #659388 - Flags: review?(fabrice)
Sets the name of the process to match the app name
Attachment #659390 - Flags: review?(jones.chris.g)
I also wrote a script, which I called b2g-ps which goes along with this. You can find it in:
https://github.com/mozilla-b2g/gonk-misc/pull/23
Comment on attachment 659390 [details] [diff] [review]
Sets the process name to match the app name

Review of attachment 659390 [details] [diff] [review]:
-----------------------------------------------------------------

Submitted try job with the following correction:
https://tbpl.mozilla.org/?tree=Try&rev=3dcea8d79b73

::: ipc/glue/ProcessUtils_none.cpp
@@ +6,5 @@
> +
> +namespace mozilla {
> +namespace ipc {
> +
> +void SetThisProcessName(const PRUinchar *aString)

That should be "const char *" rather than "const PRUichar *"
Comment on attachment 659388 [details] [diff] [review]
Adds a name field to mozIApplication

Review of attachment 659388 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nit addressed.

::: dom/interfaces/apps/mozIApplication.idl
@@ +23,5 @@
>    /* Returns the local id of the app (not the uuid used for sync). */
>    readonly attribute unsigned long localId;
> +
> +  /* Name copied from the manifest */
> +  readonly attribute DOMString name;

Nit: you also need to change the uuid of the interface.
Attachment #659388 - Flags: review?(fabrice) → review+
Comment on attachment 659390 [details] [diff] [review]
Sets the process name to match the app name

>Bug 784804 - Set the process name (comm) to the app name
>
>diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp
>--- a/dom/ipc/ContentChild.cpp
>+++ b/dom/ipc/ContentChild.cpp
>@@ -90,16 +90,17 @@
> #endif
> 
> #include "mozilla/dom/indexedDB/PIndexedDBChild.h"
> #include "mozilla/dom/sms/SmsChild.h"
> #include "mozilla/dom/devicestorage/DeviceStorageRequestChild.h"
> 
> #include "nsDOMFile.h"
> #include "nsIRemoteBlob.h"
>+#include "ProcessUtils.h"
> #include "StructuredCloneUtils.h"
> #include "URIUtils.h"
> #include "nsIScriptSecurityManager.h"
> #include "nsContentUtils.h"
> #include "nsIPrincipal.h"
> 
> using namespace base;
> using namespace mozilla::docshell;
>@@ -291,16 +292,21 @@ ContentChild::Init(MessageLoop* aIOLoop,
> 
>     bool startBackground = true;
>     SendGetProcessAttributes(&mID, &startBackground,
>                              &mIsForApp, &mIsForBrowser);
>     hal::SetProcessPriority(
>         GetCurrentProcId(),
>         startBackground ? hal::PROCESS_PRIORITY_BACKGROUND:
>                           hal::PROCESS_PRIORITY_FOREGROUND);
>+    if (mIsForApp && !mIsForBrowser) {
>+        SetThisProcessName("(App)");
>+    } else {
>+        SetThisProcessName("Browser");
>+    }
> 
>     return true;
> }
> 
> void
> ContentChild::InitXPCOM()
> {
>     nsCOMPtr<nsIConsoleService> svc(do_GetService(NS_CONSOLESERVICE_CONTRACTID));
>diff --git a/dom/ipc/TabChild.cpp b/dom/ipc/TabChild.cpp
>--- a/dom/ipc/TabChild.cpp
>+++ b/dom/ipc/TabChild.cpp
>@@ -18,21 +18,23 @@
> #include "mozilla/dom/PContentChild.h"
> #include "mozilla/dom/PContentDialogChild.h"
> #include "mozilla/ipc/DocumentRendererChild.h"
> #include "mozilla/layers/CompositorChild.h"
> #include "mozilla/layers/PLayersChild.h"
> #include "mozilla/layout/RenderFrameChild.h"
> #include "mozilla/StaticPtr.h"
> #include "mozilla/unused.h"
>+#include "mozIApplication.h"
> #include "nsComponentManagerUtils.h"
> #include "nsComponentManagerUtils.h"
> #include "nsContentUtils.h"
> #include "nsEmbedCID.h"
> #include "nsEventListenerManager.h"
>+#include "nsIAppsService.h"
> #include "nsIBaseWindow.h"
> #include "nsIComponentManager.h"
> #include "nsIDOMClassInfo.h"
> #include "nsIDOMEvent.h"
> #include "nsIDOMWindow.h"
> #include "nsIDOMWindowUtils.h"
> #include "nsIDocShell.h"
> #include "nsIDocShellTreeItem.h"
>@@ -672,16 +674,49 @@ TabChild::~TabChild()
>       nsEventListenerManager* elm = mTabChildGlobal->GetListenerManager(false);
>       if (elm) {
>         elm->Disconnect();
>       }
>       mTabChildGlobal->mTabChild = nullptr;
>     }
> }
> 
>+void
>+TabChild::SetProcessNameToAppName()
>+{
>+  if (mIsBrowserElement || (mAppId == nsIScriptSecurityManager::NO_APP_ID)) {
>+    return;
>+  }
>+  nsCOMPtr<nsIAppsService> appsService =
>+    do_GetService(APPS_SERVICE_CONTRACTID);
>+  if (!appsService) {
>+    NS_WARNING("No AppsService");
>+    return;
>+  }
>+  nsresult rv;
>+  nsCOMPtr<mozIDOMApplication> domApp;
>+  rv = appsService->GetAppByLocalId(mAppId, getter_AddRefs(domApp));
>+  if (NS_FAILED(rv) || !domApp) {
>+    NS_WARNING("GetAppByLocalId failed");
>+    return;
>+  }
>+  nsCOMPtr<mozIApplication> app = do_QueryInterface(domApp);
>+  if (!app) {
>+    NS_WARNING("app isn't a mozIApplication");
>+    return;
>+  }
>+  nsAutoString appName;
>+  rv = app->GetName(appName);
>+  if (NS_FAILED(rv)) {
>+    NS_WARNING("Failed to retrieve app name");
>+    return;
>+  }
>+  SetThisProcessName(NS_LossyConvertUTF16toASCII(appName).get());
>+}
>+
> bool
> TabChild::IsRootContentDocument()
> {
>     if (!mIsBrowserElement && mAppId == nsIScriptSecurityManager::NO_APP_ID) {
>         // We're the child side of a <xul:browser remote=true>.  This
>         // is always a root content document.
>         return true;
>     }
>@@ -693,16 +728,17 @@ TabChild::IsRootContentDocument()
>     // bug is fixed, we need to revisit that assumption.
>     return false;
> }
> 
> bool
> TabChild::RecvLoadURL(const nsCString& uri)
> {
>     printf("loading %s, %d\n", uri.get(), NS_IsMainThread());
>+    SetProcessNameToAppName();
> 
>     nsresult rv = mWebNav->LoadURI(NS_ConvertUTF8toUTF16(uri).get(),
>                                    nsIWebNavigation::LOAD_FLAGS_NONE,
>                                    NULL, NULL, NULL);
>     if (NS_FAILED(rv)) {
>         NS_WARNING("mWebNav->LoadURI failed. Eating exception, what else can I do?");
>     }
> 
>diff --git a/dom/ipc/TabChild.h b/dom/ipc/TabChild.h
>--- a/dom/ipc/TabChild.h
>+++ b/dom/ipc/TabChild.h
>@@ -43,16 +43,17 @@
> #include "nsIPrincipal.h"
> #include "nsIScriptObjectPrincipal.h"
> #include "nsIScriptContext.h"
> #include "nsPIDOMWindow.h"
> #include "nsWeakReference.h"
> #include "nsITabChild.h"
> #include "mozilla/Attributes.h"
> #include "FrameMetrics.h"
>+#include "ProcessUtils.h"
> 
> struct gfxMatrix;
> 
> namespace mozilla {
> namespace layout {
> class RenderFrameChild;
> }
> 
>@@ -304,16 +305,17 @@ private:
>     bool UseDirectCompositor();
> 
>     void ActorDestroy(ActorDestroyReason why);
> 
>     enum FrameScriptLoading { DONT_LOAD_SCRIPTS, DEFAULT_LOAD_SCRIPTS };
>     bool InitTabChildGlobal(FrameScriptLoading aScriptLoading = DEFAULT_LOAD_SCRIPTS);
>     bool InitRenderingState();
>     void DestroyWindow();
>+    void SetProcessNameToAppName();
> 
>     // Call RecvShow(nsIntSize(0, 0)) and block future calls to RecvShow().
>     void DoFakeShow();
> 
>     // Wraps up a JSON object as a structured clone and sends it to the browser
>     // chrome script.
>     //
>     // XXX/bug 780335: Do the work the browser chrome script does in C++ instead
>diff --git a/ipc/glue/Makefile.in b/ipc/glue/Makefile.in
>--- a/ipc/glue/Makefile.in
>+++ b/ipc/glue/Makefile.in
>@@ -100,16 +100,22 @@ CPPSRCS += \
>   CrossProcessMutex_unimplemented.cpp \
>   $(NULL)
> endif #}
> 
> ifeq ($(OS_TARGET),Android)
> CPPSRCS += SharedMemoryBasic_android.cpp
> endif #}
> 
>+ifeq ($(OS_ARCH),Linux)
>+CPPSRCS += ProcessUtils_linux.cpp
>+else
>+CPPSRCS += ProcessUtils_none.cpp
>+endif
>+
> include $(topsrcdir)/ipc/app/defs.mk
> DEFINES += -DMOZ_CHILD_PROCESS_NAME=\"$(MOZ_CHILD_PROCESS_NAME)\"
> DEFINES += -DMOZ_CHILD_PROCESS_BUNDLE=\"$(MOZ_CHILD_PROCESS_BUNDLE)\"
> 
> include $(topsrcdir)/config/config.mk
> include $(topsrcdir)/ipc/chromium/chromium-config.mk
> 
> include $(topsrcdir)/config/rules.mk
>diff --git a/ipc/glue/ProcessUtils.h b/ipc/glue/ProcessUtils.h
>new file mode 100644
>--- /dev/null
>+++ b/ipc/glue/ProcessUtils.h
>@@ -0,0 +1,17 @@
>+/* 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/. */
>+
>+#ifndef mozilla_ipc_ProcessUtils_h
>+#define mozilla_ipc_ProcessUtils_h
>+
>+namespace mozilla {
>+namespace ipc {
>+
>+void SetThisProcessName(const char *aName);
>+
>+} // namespace ipc
>+} // namespace mozilla
>+
>+#endif // ifndef mozilla_ipc_ProcessUtils_h
>+
>diff --git a/ipc/glue/ProcessUtils_linux.cpp b/ipc/glue/ProcessUtils_linux.cpp
>new file mode 100644
>--- /dev/null
>+++ b/ipc/glue/ProcessUtils_linux.cpp
>@@ -0,0 +1,20 @@
>+/* 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 "ProcessUtils.h"
>+
>+#include "nsString.h"
>+
>+#include <sys/prctl.h>
>+
>+namespace mozilla {
>+namespace ipc {
>+
>+void SetThisProcessName(const char *aName)
>+{
>+  prctl(PR_SET_NAME, (unsigned long)aName, 0uL, 0uL, 0uL);
>+}
>+
>+} // namespace ipc
>+} // namespace mozilla
>diff --git a/ipc/glue/ProcessUtils_none.cpp b/ipc/glue/ProcessUtils_none.cpp
>new file mode 100644
>--- /dev/null
>+++ b/ipc/glue/ProcessUtils_none.cpp
>@@ -0,0 +1,16 @@
>+/* 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 "ProcessUtils.h"
>+
>+namespace mozilla {
>+namespace ipc {
>+
>+void SetThisProcessName(const PRUinchar *aString)
>+{
>+  (void)aString;
>+}
>+
>+} // namespace ipc
>+} // namespace mozilla
Attachment #659390 - Flags: review?(jones.chris.g) → review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: