Add helper to generate a globally unique ID / process ID

RESOLVED FIXED in Firefox 51

Status

WebExtensions
Untriaged
RESOLVED FIXED
2 years ago
a month ago

People

(Reporter: robwu, Assigned: robwu)

Tracking

Trunk
mozilla51
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox50 affected, firefox51 fixed)

Details

(Whiteboard: triaged)

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Bug 1287245 is caused by the incorrect assumption that an incrementing global JS variable is unique, even across processes.
To fix this, we need a way to obtain an ID that is guaranteed to be unique regardless of the process.

:billm suggests to publish the child ID @ http://searchfox.org/mozilla-central/rev/bfcc10319e4e3ce78367fa9bba9316f7eb5248b6/dom/ipc/ContentParent.cpp#2174 via nsIMessageManager.idl.

Updated

2 years ago
Blocks: 905436
tracking-e10s: ? → +

Updated

2 years ago
Whiteboard: triaged
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Attachment #8779921 - Flags: review?(wmccloskey)
Attachment #8779922 - Flags: review?(wmccloskey)
(Assignee)

Updated

2 years ago
Blocks: 1288279
Comment hidden (mozreview-request)
Comment on attachment 8779921 [details]
Bug 1287626 - Add globally unique nsIXULAppinfo.uniqueProcessID

https://reviewboard.mozilla.org/r/70808/#review68534

::: xpcom/system/nsIXULRuntime.idl:90
(Diff revision 1)
>     * The system process ID of the caller's process.
>     */
>    readonly attribute unsigned long processID;
>  
>    /**
> +   * A unique ID of the caller's process, globally unique across processes.

Please mention that these IDs are not recycled, unlike the "processID" attribute above it.

::: xpcom/system/nsIXULRuntime.idl:92
(Diff revision 1)
>    readonly attribute unsigned long processID;
>  
>    /**
> +   * A unique ID of the caller's process, globally unique across processes.
> +   */
> +  readonly attribute unsigned long uniqueProcessID;

Please make this an unsigned long long. Then the C++ type will be uint64_t. We'll only get about 56 bits in JS, but that's fine.
Attachment #8779921 - Flags: review?(wmccloskey) → review+
Comment on attachment 8779922 [details]
Bug 1287626,1288279 - Make IDs unique across processes

https://reviewboard.mozilla.org/r/70810/#review68536

::: toolkit/components/extensions/ExtensionUtils.jsm:152
(Diff revision 2)
>  class BaseContext {
>    constructor(extensionId) {
>      this.onClose = new Set();
>      this.checkedLastError = false;
>      this._lastError = null;
> -    this.contextId = ++gContextId;
> +    this.contextId = `${++gContextId}${Services.appinfo.uniqueProcessID}`;

We should have a dash in between the two IDs to avoid confusion if, for example, ++gContextId is 1 and the process ID is 12 versus ++gContextId is 11 and the process ID is 2. Same for the other ones too.
Attachment #8779922 - Flags: review?(wmccloskey) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6e0577a5a59a
Add globally unique nsIXULAppinfo.uniqueProcessID r=billm
https://hg.mozilla.org/integration/autoland/rev/a327a15f5d55
1288279 - Make IDs unique across processes r=billm
Keywords: checkin-needed

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6e0577a5a59a
https://hg.mozilla.org/mozilla-central/rev/a327a15f5d55
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51

Updated

a month ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.