1333277 - [multi-e10s] add pid of a tab to tooltip

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: pid_tooltip.patch

It would be useful for debugging issues such as tab spinners if we could know the pid that is associated with a given tab.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Note, this fails the linter because of the nested ternary. I don't understand why that's not allowed

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8829712 [details] [diff] [review]
pid_tooltip.patch

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

Looks okay - but I think we can break this up to be a little more easily readable (and satisfy the linter while we're at it).

::: browser/base/content/tabbrowser.xml
@@ +4669,5 @@
>                label = this.mStringBundle.getString(stringID);
>              }
>            } else {
>              label = tab.getAttribute("label") +
> +                      (this.AppConstants.E10S_TESTING_ONLY && tab.linkedBrowser && tab.linkedBrowser.isRemoteBrowser ? (Services.prefs.getIntPref("dom.ipc.processCount") > 1 ? " - e10s (" + tab.linkedBrowser.frameLoader.tabParent.osPid + ")" : " - e10s"  ) : "");

I worry that this is getting a little unwieldy. Nested ternary is hard to read, which is why the linter is failing here.

Perhaps we can break this up, like:

label = this.getAttribute("label");

if (this.AppConstants.E10S_TESTING_ONLY &&
    tab.linkedBrowser &&
    tab.linkedBrowser.isRemoteBrowser) {

  label += " - e10s";

  if (Services.prefs.getIntPref("dom.ipc.processCount") > 1) {
    label += ` (${tab.linkedBrowser.frameLoader.tabParent.osPid})`;
  }
}

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3613ce87bd3d
add pid of tab to tooltip r=mconley

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3613ce87bd3d