https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/gpu/d...
File ui/ozone/platform/drm/gpu/drm_thread.h (right):
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/gpu/d...
ui/ozone/platform/drm/gpu/drm_thread.h:85: void MoveCursor(const
gfx::AcceleratedWidget& widget,
On 2016/06/21 17:20:52, rjkroege wrote:
> On 2016/06/21 14:14:16, dnicoara wrote:
> > nit: Why const& for AcceleratedWidget? It is supposed to be lightweight.
>
> To permit the mojo struct traits implementation to be zero-copy, it has to be
> const&. You will be seeing more of this for the same reason in subsequent CLs.
Ah, good to know, thanks for the explanation. Out of curiosity is there a mojo
traits dos/don'ts guide?
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/host/...
File ui/ozone/platform/drm/host/drm_cursor.cc (right):
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/host/...
ui/ozone/platform/drm/host/drm_cursor.cc:216:
evdev_thread_checker_.DetachFromThread();
On 2016/06/21 17:20:52, rjkroege wrote:
> On 2016/06/21 14:14:16, dnicoara wrote:
> > Unless this is not called on the evdev thread you shouldn't detach. (In fact
> you
> > should DCHECK it since that makes sure |evdev_tread_checker_| is attached to
> the
> > correct thread)
>
> The evdev_thread_checker_ is initialized on the main thread in the
constructor.
> So, I need to call DetachFromThread here to force the thread checker to be
> attached to the evdev thread instead yes? This is true empirically: the thread
> where this class is constructed is most definitely not the same thread as
where
> InitializeOnEvdev is called.
Ah, I see your reasoning. My understanding is that you should then call
DetachFromThread() when |evdev_thread_checker_| is created (in DrmCursor
constructor) and call CalledOnValidThread() in here. That way all call sites
that need to be done on the Evdev thread are checked.

On 2016/06/21 17:38:18, dnicoara wrote:
>
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/gpu/d...
> File ui/ozone/platform/drm/gpu/drm_thread.h (right):
>
>
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/gpu/d...
> ui/ozone/platform/drm/gpu/drm_thread.h:85: void MoveCursor(const
> gfx::AcceleratedWidget& widget,
> On 2016/06/21 17:20:52, rjkroege wrote:
> > On 2016/06/21 14:14:16, dnicoara wrote:
> > > nit: Why const& for AcceleratedWidget? It is supposed to be lightweight.
> >
> > To permit the mojo struct traits implementation to be zero-copy, it has to
be
> > const&. You will be seeing more of this for the same reason in subsequent
CLs.
>
> Ah, good to know, thanks for the explanation. Out of curiosity is there a mojo
> traits dos/don'ts guide?
https://www.chromium.org/developers/design-documents/mojo/type-mapping is the
best guide so far that I've seen.
>
>
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/host/...
> File ui/ozone/platform/drm/host/drm_cursor.cc (right):
>
>
https://codereview.chromium.org/2088533002/diff/1/ui/ozone/platform/drm/host/...
> ui/ozone/platform/drm/host/drm_cursor.cc:216:
> evdev_thread_checker_.DetachFromThread();
> On 2016/06/21 17:20:52, rjkroege wrote:
> > On 2016/06/21 14:14:16, dnicoara wrote:
> > > Unless this is not called on the evdev thread you shouldn't detach. (In
fact
> > you
> > > should DCHECK it since that makes sure |evdev_tread_checker_| is attached
to
> > the
> > > correct thread)
> >
> > The evdev_thread_checker_ is initialized on the main thread in the
> constructor.
> > So, I need to call DetachFromThread here to force the thread checker to be
> > attached to the evdev thread instead yes? This is true empirically: the
thread
> > where this class is constructed is most definitely not the same thread as
> where
> > InitializeOnEvdev is called.
>
> Ah, I see your reasoning. My understanding is that you should then call
> DetachFromThread() when |evdev_thread_checker_| is created (in DrmCursor
> constructor) and call CalledOnValidThread() in here. That way all call sites
> that need to be done on the Evdev thread are checked.
Ah. Nice. Done that way.