Linux Aura: Fixes and improvements for cursor loader

ui/base/cursor/cursor_loader_x11.h

Index: ui/base/cursor/cursor_loader_x11.h
diff --git a/ui/base/cursor/cursor_loader_x11.h b/ui/base/cursor/cursor_loader_x11.h
index 582a61538abc0ca6f091bec4dff1c4568626caf6..5aa127d214fd710ebbe4e1e0d6a433d2afc6ff01 100644
--- a/ui/base/cursor/cursor_loader_x11.h
+++ b/ui/base/cursor/cursor_loader_x11.h
@@ -6,7 +6,7 @@
 #define UI_BASE_CURSOR_CURSOR_LOADER_X11_H_
 
 #include <X11/Xcursor/Xcursor.h>
-#include <map>
+#include <unordered_map>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -34,20 +34,37 @@ class UI_BASE_EXPORT CursorLoaderX11 : public CursorLoader {
   const XcursorImage* GetXcursorImageForTest(int id);
 
  private:
+  struct ImageCursor {
+    ImageCursor(XcursorImage* x_image,
+                float scale,
+                display::Display::Rotation rotation);
+    ~ImageCursor();
+
+    ::Cursor cursor;
+    float scale;
+    display::Display::Rotation rotation;

[sadrul, 2016/11/17 16:17:03]

In multi-display, different displays can have different values for
scale/rotation. Do we create the cursors separately for each display?

[Tom (Use chromium acct), 2016/11/17 19:17:48]

Image cursors are loaded from here:
https://cs.chromium.org/chromium/src/ui/base/cursor/image_cursors.cc?rcl=1479...
So it would appear so?

But I did some debugging and found that there's only 1 cursor loader in the
multi monitor case.  I tested rotating screens while chrome was running and all
image cursors behaved well, but I'm not sure I understand why it's working.

At any rate, I don't think black-box behavior of CursorLoaderX11 is changed.

[sadrul, 2016/11/18 01:27:30]

Do you mean with your change, or without your change?

It looks like you effectively reload the image cursors (on-demand) when the
scale/rotation changes of a display (in CursorLoaderX11::CursorFromId()). If we
don't actually need to do that, then we shouldn't do that.

+  };
+
   // Returns true if we have an image resource loaded for the |native_cursor|.
   bool IsImageCursor(gfx::NativeCursor native_cursor);
 
-  // Gets the X Cursor corresponding to the |native_cursor|.
-  ::Cursor ImageCursorFromNative(gfx::NativeCursor native_cursor);
+  // Loads a new cursor corresponding to |id|.
+  ::Cursor CursorFromId(int id);
 
-  // A map to hold all image cursors. It maps the cursor ID to the X Cursor.
-  typedef std::map<int, ::Cursor> ImageCursorMap;
-  ImageCursorMap cursors_;
+  XDisplay* display_;
+
+  // A map from a cursor native type to X cursor.
+  std::unordered_map<int, ::Cursor> font_cursors_;
+
+  // A map to hold all image cursors. It maps the cursor ID to the X Cursor, the
+  // display's scale factor, and the display's rotation.
+  typedef std::unordered_map<int, std::unique_ptr<ImageCursor>> ImageCursorMap;
+  ImageCursorMap image_cursors_;
 
   // A map to hold all animated cursors. It maps the cursor ID to the pair of
   // the X Cursor and the corresponding XcursorImages. We need a pointer to the
   // images so that we can free them on destruction.
-  typedef std::map<int, std::pair< ::Cursor, XcursorImages*> >
+  typedef std::unordered_map<int, std::pair<::Cursor, XcursorImages*>>
       AnimatedCursorMap;
   AnimatedCursorMap animated_cursors_;