Expose a way to obtain a unique ID for the current process.

Expose a way to obtain a unique ID for the current process.

BUG=501069, 499332
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197514

[rickyz (no longer on Chrome), 2015-06-17 21:09:32]

rickyz@chromium.org changed reviewers:
+ dcheng@chromium.org

[rickyz (no longer on Chrome), 2015-06-17 21:09:33]

Hi, this is the start of the fix for issue 499332 - the problem is that people
were using getpid() as a unique identifier for a process in some webkit code,
but under PID namespaces on Linux, getpid() returns 1 in all renderers. The plan
is to add a Platform.h method that we can implement on the chromium side, which
has knowledge of the PID outside the namespace.

[rickyz (no longer on Chrome), 2015-06-17 21:12:49]

On 2015/06/17 21:09:33, rickyz wrote:
> Hi, this is the start of the fix for issue 499332 - the problem is that people
> were using getpid() as a unique identifier for a process in some webkit code,
> but under PID namespaces on Linux, getpid() returns 1 in all renderers. The
plan
> is to add a Platform.h method that we can implement on the chromium side,
which
> has knowledge of the PID outside the namespace.

(oh, and FYI, the chrome side looks like this:
https://codereview.chromium.org/1186933004/)

[dcheng, 2015-06-17 23:05:38]

Hmm...

Is there ever a reason we'd want to use the getpid() stuff in Blink at all? I
see a couple other random usages of it here and there
(Source/web/WebDevToolsAgentImpl.cpp, Source/wtf/text/StringImpl.cpp,
Source/wtf/DataLog.cpp)... could we consolidate all these and add a presubmit
warning?

[rickyz (no longer on Chrome), 2015-06-17 23:16:07]

On 2015/06/17 23:05:38, dcheng wrote:
> Hmm...
> 
> Is there ever a reason we'd want to use the getpid() stuff in Blink at all? I
> see a couple other random usages of it here and there
> (Source/web/WebDevToolsAgentImpl.cpp, Source/wtf/text/StringImpl.cpp,
> Source/wtf/DataLog.cpp)... could we consolidate all these and add a presubmit
> warning?

My plan is to fully rip out WTF::getCurrentProcessID once getUniqueIdForProcess
is a viable alternative. The only important usage is WebDevToolsAgentImpl, which
functionally relies on the uniqueness of this id - all the others look to be in
dead or debugging code from what I can tell.

Unfortunately, the blink/chromium split prevents me from doing all the changes
in one shot - I was going to submit this one, the chromium one, then one more
blink one that switches all of the uses over.

[rickyz (no longer on Chrome), 2015-06-19 23:04:41]

On 2015/06/17 23:16:07, rickyz wrote:
> On 2015/06/17 23:05:38, dcheng wrote:
> > Hmm...
> > 
> > Is there ever a reason we'd want to use the getpid() stuff in Blink at all?
I
> > see a couple other random usages of it here and there
> > (Source/web/WebDevToolsAgentImpl.cpp, Source/wtf/text/StringImpl.cpp,
> > Source/wtf/DataLog.cpp)... could we consolidate all these and add a
presubmit
> > warning?
> 
> My plan is to fully rip out WTF::getCurrentProcessID once
getUniqueIdForProcess
> is a viable alternative. The only important usage is WebDevToolsAgentImpl,
which
> functionally relies on the uniqueness of this id - all the others look to be
in
> dead or debugging code from what I can tell.
> 
> Unfortunately, the blink/chromium split prevents me from doing all the changes
> in one shot - I was going to submit this one, the chromium one, then one more
> blink one that switches all of the uses over.

Friendly ping :-)

[dcheng, 2015-06-19 23:38:05]

lgtm (sorry I missed your followup reply 2 days ago; feel free to ping me more
aggressively)

[rickyz (no longer on Chrome), 2015-06-19 23:38:34]

The CQ bit was checked by rickyz@chromium.org

[commit-bot: I haz the power, 2015-06-19 23:39:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1181603007/1

[commit-bot: I haz the power, 2015-06-20 01:02:48]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197514

[rickyz (no longer on Chrome), 2015-06-23 23:52:22]

Patchset #2 (id:20001) has been deleted