[3/4][Ozone-Drm] Use numeric ID to map to callback

[Ozone-Drm] Use numeric ID to map to callback

In order to allow sharing the DRM connection between browser and GPU
process, we need to be able to recover from GPU crashes and gracefully
handle page flipping events that were meant for the previous GPU
process.

BUG=445773
Committed: https://crrev.com/c7889fcaadb851ddf171333a3fa94c59f79e4d1c
Cr-Commit-Position: refs/heads/master@{#327527}

[dnicoara, 2015-04-27 14:21:43]

dnicoara@chromium.org changed reviewers:
+ spang@chromium.org

[spang, 2015-04-27 17:23:39]

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
File ui/ozone/platform/drm/gpu/drm_device.cc (right):

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:94: drm_event* event =
reinterpret_cast<drm_event*>(&buffer[idx]);
You can't cast a char* to an arbitrary struct. It's undefined behavior.

You can memcpy() into a drm_event instead.

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:153: std::map<uint64_t,
DrmDevice::PageFlipCallback> callback_map_;
vector?

[dnicoara, 2015-04-27 20:22:22]

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
File ui/ozone/platform/drm/gpu/drm_device.cc (right):

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:94: drm_event* event =
reinterpret_cast<drm_event*>(&buffer[idx]);
On 2015/04/27 17:23:38, spang wrote:
> You can't cast a char* to an arbitrary struct. It's undefined behavior.
> 

Why would it be undefined behavior to cast between 2 pointer types? Are you
afraid of bounds checks? DRM is supposed to guarantee complete events, but I can
add some checks.

> You can memcpy() into a drm_event instead.

Seems silly to copy the event twice.

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:153: std::map<uint64_t,
DrmDevice::PageFlipCallback> callback_map_;
On 2015/04/27 17:23:38, spang wrote:
> vector?

Why vector? ... I suppose the number of callbacks registered won't be large, but
this is easier to read.

[spang, 2015-04-27 20:53:42]

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
File ui/ozone/platform/drm/gpu/drm_device.cc (right):

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:153: std::map<uint64_t,
DrmDevice::PageFlipCallback> callback_map_;
On 2015/04/27 20:22:22, dnicoara wrote:
> On 2015/04/27 17:23:38, spang wrote:
> > vector?
> 
> Why vector? ... I suppose the number of callbacks registered won't be large,
but
> this is easier to read.

You'll only have 1-3 elements right? vector is a better choice in that case.

[dnicoara, 2015-04-28 14:42:49]

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
File ui/ozone/platform/drm/gpu/drm_device.cc (right):

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:94: drm_event* event =
reinterpret_cast<drm_event*>(&buffer[idx]);
On 2015/04/27 17:23:38, spang wrote:
> You can't cast a char* to an arbitrary struct. It's undefined behavior.
> 
> You can memcpy() into a drm_event instead.

Done.

https://codereview.chromium.org/1056793007/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_device.cc:153: std::map<uint64_t,
DrmDevice::PageFlipCallback> callback_map_;
On 2015/04/27 20:53:42, spang wrote:
> On 2015/04/27 20:22:22, dnicoara wrote:
> > On 2015/04/27 17:23:38, spang wrote:
> > > vector?
> > 
> > Why vector? ... I suppose the number of callbacks registered won't be large,
> but
> > this is easier to read.
> 
> You'll only have 1-3 elements right? vector is a better choice in that case.

Done.

https://codereview.chromium.org/1056793007/diff/20001/ui/ozone/platform/drm/g...
File ui/ozone/platform/drm/gpu/drm_device.cc (right):

https://codereview.chromium.org/1056793007/diff/20001/ui/ozone/platform/drm/g...
ui/ozone/platform/drm/gpu/drm_device.cc:252: UnregisterOnIO();
Also, since it was a trivial change, unregistered the FD if failed processing.

[spang, 2015-04-28 15:13:37]

lgtm

[dnicoara, 2015-04-29 17:47:23]

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

[commit-bot: I haz the power, 2015-04-29 17:47:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1056793007/40001

[commit-bot: I haz the power, 2015-04-29 18:15:34]

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

[commit-bot: I haz the power, 2015-04-29 18:15:35]

Dry run: This issue passed the CQ dry run.

[dnicoara, 2015-04-29 18:15:55]

The CQ bit was checked by dnicoara@chromium.org

[commit-bot: I haz the power, 2015-04-29 18:17:21]

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

[commit-bot: I haz the power, 2015-04-29 18:18:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-29 18:20:48]

Patchset 3 (id:??) landed as
https://crrev.com/c7889fcaadb851ddf171333a3fa94c59f79e4d1c
Cr-Commit-Position: refs/heads/master@{#327527}