[Remoting Android] Fix thread issue with OpenGL drawable

remoting/client/display/gl_cursor.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef REMOTING_CLIENT_DISPLAY_GL_CURSOR_H_
 #define REMOTING_CLIENT_DISPLAY_GL_CURSOR_H_
 
 #include <array>
 #include <cstdint>
 #include <memory>
@@ skipping 43 matching lines @@
   std::unique_ptr<uint8_t[]> current_cursor_data_;
   int current_cursor_data_size_ = 0;
   int current_cursor_width_ = 0;
   int current_cursor_height_ = 0;
   int current_cursor_hotspot_x_ = 0;
   int current_cursor_hotspot_y_ = 0;
 
   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);
 };
 
 }  // namespace remoting
 #endif  // REMOTING_CLIENT_DISPLAY_GL_CURSOR_H_