Don't fetch histograms from the same process.

Don't fetch histograms from the same process.

This change makes HistogramController not fetch any
histograms from a "child" process when that process
is the same as the current process. In reality, this
could be the case on Android where the browser
process is actually also the GPU process.

See associated bug for the specific issue this is
actually fixing.

Also updates the profiler child process code to
use the same logic (it was not affected by the
bug, though).

BUG=370208
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276900

[Alexei Svitkine (slow), 2014-06-10 19:52:15]

Don't review yet, since apparently it doesn't work. Investigating.

[Alexei Svitkine (slow), 2014-06-11 05:29:49]

+piman for OWNERS review

David, can you try out the latest patchset and let me know it fixes the issue?

[bokan, 2014-06-11 13:25:14]

On 2014/06/11 05:29:49, Alexei Svitkine wrote:
> +piman for OWNERS review
> 
> David, can you try out the latest patchset and let me know it fixes the issue?

Yup, looks like it works, thanks!

[piman, 2014-06-11 19:46:36]

https://codereview.chromium.org/328823002/diff/100001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/328823002/diff/100001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host.cc:352:
process_->SetHandle(base::GetCurrentProcessHandle());
Seems iffy... It will have side effects in other places (e.g.
OnChildDisconnected). Why not leaving it as kNullProcessHandle and test that
instead?

[Alexei Svitkine (slow), 2014-06-11 19:52:29]

https://codereview.chromium.org/328823002/diff/100001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/328823002/diff/100001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host.cc:352:
process_->SetHandle(base::GetCurrentProcessHandle());
On 2014/06/11 19:46:35, piman wrote:
> Seems iffy... It will have side effects in other places (e.g.
> OnChildDisconnected). Why not leaving it as kNullProcessHandle and test that
> instead?

I'm okay with that solution too. We can expand the documentation for the
|handle| field to mention what kNullProcessHandle may mean. As far as I could
tell, I think the correct description for it would be:

// The handle to the process. May have value kNullProcessHandle if no process
// exists - either because it hasn't been started yet or it's running in the
// current process.

Then, in the other files I can check for kNullProcessHandle. Do you prefer this
approach?

[piman, 2014-06-12 17:20:30]

https://codereview.chromium.org/328823002/diff/100001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_process_host.cc (right):

https://codereview.chromium.org/328823002/diff/100001/content/browser/gpu/gpu...
content/browser/gpu/gpu_process_host.cc:352:
process_->SetHandle(base::GetCurrentProcessHandle());
On 2014/06/11 19:52:28, Alexei Svitkine wrote:
> On 2014/06/11 19:46:35, piman wrote:
> > Seems iffy... It will have side effects in other places (e.g.
> > OnChildDisconnected). Why not leaving it as kNullProcessHandle and test that
> > instead?
> 
> I'm okay with that solution too. We can expand the documentation for the
> |handle| field to mention what kNullProcessHandle may mean. As far as I could
> tell, I think the correct description for it would be:
> 
> // The handle to the process. May have value kNullProcessHandle if no process
> // exists - either because it hasn't been started yet or it's running in the
> // current process.
> 
> Then, in the other files I can check for kNullProcessHandle. Do you prefer
this
> approach?

Yes... I think the BrowserChildProcessHost's handle needs to accurately
represent the child process it owns, not the process that created it. If no
process was launched, then the handle is NULL.

[Alexei Svitkine (slow), 2014-06-12 17:30:51]

All right, CL updated. PTAL.

bokan@ can you sanity-check that this fix still works? Thanks!

[piman, 2014-06-12 18:13:07]

lgtm

[bokan, 2014-06-12 18:28:04]

On 2014/06/12 17:30:51, Alexei Svitkine wrote:
> All right, CL updated. PTAL.
> 
> bokan@ can you sanity-check that this fix still works? Thanks!

Confirmed, still works.

[Ilya Sherman, 2014-06-12 20:24:02]

LGTM.  Please file a bug to follow up on whether we can now remove the
kIPCSerializationSourceFlag.

[Alexei Svitkine (slow), 2014-06-12 20:25:29]

On 2014/06/12 20:24:02, Ilya Sherman wrote:
> LGTM.  Please file a bug to follow up on whether we can now remove the
> kIPCSerializationSourceFlag.

Done. http://crbug.com/384020

[Alexei Svitkine (slow), 2014-06-12 20:25:47]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2014-06-12 20:28:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/asvitkine@chromium.org/328823002/120001

[commit-bot: I haz the power, 2014-06-13 02:44:56]

Change committed as 276900