gpu: Use a local int to generate the gmb id.

gpu: Use a local int to generate the gmb id.

For gpu memory buffers, use a local int in BrowserGpuMemoryBufferManager
to generate the id, instead of a global base::StaticAtomicSequenceNumber.

BUG=none

Review-Url: https://codereview.chromium.org/2963813002
Cr-Commit-Position: refs/heads/master@{#483591}
Committed: https://chromium.googlesource.com/chromium/src/+/646ad10da49cdb8ed89c64462dde3e95142c36a8

[sadrul, 2017-06-28 17:22:21]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-28 17:22:35]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2017-06-28 17:23:22]

sadrul@chromium.org changed reviewers:
+ reveman@chromium.org

[commit-bot: I haz the power, 2017-06-28 18:36:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-28 18:36:48]

Dry run: This issue passed the CQ dry run.

[reveman, 2017-06-28 19:00:33]

why not gfx::GenericSharedMemoryId anymore? Is this not going to cause
memory-infra problems as that was the reason we used this..

[sadrul, 2017-06-28 19:36:19]

On 2017/06/28 19:00:33, reveman wrote:
> why not gfx::GenericSharedMemoryId anymore? Is this not going to cause
> memory-infra problems as that was the reason we used this..

Not totally sure what you mean. We are still using GpuMemoryBufferId (which is-a
GenericSharedMemoryId [1]). So this should be OK?

[1] https://cs.chromium.org/chromium/src/ui/gfx/gpu_memory_buffer.h?l=39

[reveman, 2017-06-28 22:59:41]

On 2017/06/28 at 19:36:19, sadrul wrote:
> On 2017/06/28 19:00:33, reveman wrote:
> > why not gfx::GenericSharedMemoryId anymore? Is this not going to cause
> > memory-infra problems as that was the reason we used this..
> 
> Not totally sure what you mean. We are still using GpuMemoryBufferId (which
is-a GenericSharedMemoryId [1]). So this should be OK?
> 
> [1] https://cs.chromium.org/chromium/src/ui/gfx/gpu_memory_buffer.h?l=39

Maybe fine. What I'm worried about is that we'll have an issue when some other
shared memory instance ends up with the same id as a GMB instance but maybe
that's not a problem?

[sadrul, 2017-06-28 23:33:53]

On 2017/06/28 22:59:41, reveman wrote:
> On 2017/06/28 at 19:36:19, sadrul wrote:
> > On 2017/06/28 19:00:33, reveman wrote:
> > > why not gfx::GenericSharedMemoryId anymore? Is this not going to cause
> > > memory-infra problems as that was the reason we used this..
> > 
> > Not totally sure what you mean. We are still using GpuMemoryBufferId (which
> is-a GenericSharedMemoryId [1]). So this should be OK?
> > 
> > [1] https://cs.chromium.org/chromium/src/ui/gfx/gpu_memory_buffer.h?l=39
> 
> Maybe fine. What I'm worried about is that we'll have an issue when some other
> shared memory instance ends up with the same id as a GMB instance but maybe
> that's not a problem?

This is the only caller of content::GetNextGenericSharedMemoryId(), so I think
if it happens, it can happen both with or without this change?

[reveman, 2017-06-29 00:27:54]

reveman@chromium.org changed reviewers:
+ ericrk@chromium.org

[reveman, 2017-06-29 00:27:55]

It was added by
https://chromium.googlesource.com/chromium/src/+/c9984ebe42daab4cd51fb928c32d...
and removing it lgtm unless we feel like having this is still a good idea.
+ericrk

[sadrul, 2017-06-29 02:34:36]

On 2017/06/29 00:27:55, reveman wrote:
> It was added by
>
https://chromium.googlesource.com/chromium/src/+/c9984ebe42daab4cd51fb928c32d...
> and removing it lgtm unless we feel like having this is still a good idea.
> +ericrk

I think with that CL, content::GetNextGenericSharedMemoryId() could be called in
the renderer too, and from any thread [1]. However, in the current code, that
function is no longer used, since all the calls go through the same thread [2].
In case of the browser, it is also always called on the same thread (IO) [3]. So
I don't think using an AtomicSequenceNumber currently gives us any benefits?

[1]
https://chromium.googlesource.com/chromium/src/+/c9984ebe42daab4cd51fb928c32d...
[2]
https://cs.chromium.org/chromium/src/services/ui/public/cpp/gpu/client_gpu_me...
[3]
https://cs.chromium.org/chromium/src/content/browser/gpu/browser_gpu_memory_b...

[sadrul, 2017-06-29 19:55:29]

sadrul@chromium.org changed reviewers:
+ piman@chromium.org

[sadrul, 2017-06-29 19:55:29]

+piman@ for owner review.

[piman, 2017-06-29 20:23:24]

lgtm

[ericrk, 2017-06-29 22:17:07]

LGTM as well - we were using this for both GMBs and other shared memory types,
but recent refactoring got rid of the other use cases. Thanks for cleaning this
up.

[sadrul, 2017-06-30 00:40:17]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2017-06-30 00:40:43]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-30 02:04:56]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1498783217256520, "parent_rev":
"5573f5319343ee797ac35b67006b8ba04afb648d", "commit_rev":
"646ad10da49cdb8ed89c64462dde3e95142c36a8"}

[commit-bot: I haz the power, 2017-06-30 02:05:10]

Description was changed from

==========
gpu: Use a local int to generate the gmb id.

For gpu memory buffers, use a local int in BrowserGpuMemoryBufferManager
to generate the id, instead of a global base::StaticAtomicSequenceNumber.

BUG=none
==========

to

==========
gpu: Use a local int to generate the gmb id.

For gpu memory buffers, use a local int in BrowserGpuMemoryBufferManager
to generate the id, instead of a global base::StaticAtomicSequenceNumber.

BUG=none

Review-Url: https://codereview.chromium.org/2963813002
Cr-Commit-Position: refs/heads/master@{#483591}
Committed:
https://chromium.googlesource.com/chromium/src/+/646ad10da49cdb8ed89c64462dde...
==========

[commit-bot: I haz the power, 2017-06-30 02:05:12]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/646ad10da49cdb8ed89c64462dde...