The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Firefox 54

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: blassey, Unassigned)

Tracking

unspecified
Firefox 54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(1 attachment)

Created attachment 8829712 [details] [diff] [review]
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.
Attachment #8829712 - Flags: review?(mconley)
(Reporter)

Comment 1

2 months ago
Note, this fails the linter because of the nested ternary. I don't understand why that's not allowed
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})`;
  }
}
Attachment #8829712 - Flags: review?(mconley) → review+

Comment 3

2 months ago
Pushed by blassey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3613ce87bd3d
add pid of tab to tooltip r=mconley

Comment 4

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3613ce87bd3d
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.