[Remoting Android] Fix thread issue with OpenGL drawable

remoting/client/display/gl_cursor.h

Index: remoting/client/display/gl_cursor.h
diff --git a/remoting/client/display/gl_cursor.h b/remoting/client/display/gl_cursor.h
index 07997e064b649ed363044e84989cad06b3f437c3..9751f6f24ba97f026bd9efc45413a180636180da 100644
--- a/remoting/client/display/gl_cursor.h
+++ b/remoting/client/display/gl_cursor.h
@@ -61,7 +61,7 @@ class GlCursor : public Drawable {
   float cursor_x_ = 0;
   float cursor_y_ = 0;
 
-  base::ThreadChecker thread_checker_;

[Yuwei, 2017/01/13 01:29:00]

Removed thread_checker_ entirely since it's only used in Draw() but not in other
functions with side effect. Not sure what's the point of it...

[Sergey Ulanov, 2017/01/13 02:03:10]

It still can be useful, even if used in one place:
 1. It verifies that Draw() is called on the same thread that crated the object.
 2. It verifies that the object is destroyed on the same thread on which it's
created.

But I don't think we have thread_checker_ in every single-threaded class, so I
don't mind removing it here.

[Sergey Ulanov, 2017/01/13 02:06:19]

Also, why is it called only in Draw()? Is there any case when these functions
are called on threads other than the display thread?

[Yuwei, 2017/01/13 02:21:08]

I don't know... Scott may know better O_o
No. It should be entirely on Display. Previously I didn't bother introducing a
thread checker since GlRenderer already thread-checks all functions.

[joedow, 2017/01/13 16:09:34]

Threadchecker was added to ensure the WeakPtr() was created on the same thread
the class was created on.  It could have been added to every method but was not
done in the original CL.

WeakPtrs are only guaranteed to be valid if they are used on the thread they are
bound to, if you pass one to another thread, it is possible for it to be out of
sync if it is invalidated on the original thread.

Is the design such that the GL objects are created and used on a single thread
or does it invlove multiple threads?

[nicholss, 2017/01/13 16:45:01]

For the current usage, these classes could get away with just assuming it is
being called on the network thread but that is because this system was designed
highly coupled and my goals have been to break up the graph and allow other
classes to add and control the drawable elements.

For example: the renderer should not be the class that delegates the drawing of
the decoded frames to the desktop. It should be the direct responsibility of the
desktop or related class to push that to the desktop object and then the desktop
object should request a render cycle from the renderer. The renderer should not
be tied to the implementation of chromoting at all. So in this future, these
classes are independent from the renderer and potentially getting called from a
thread that is not the rendering thread, and thread checker is a good idea.

+  base::WeakPtr<Drawable> weak_ptr_;
   base::WeakPtrFactory<Drawable> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(GlCursor);