Display Window Icon In Picker UI

chrome/browser/media/window_icon_util_x11.cc

 // 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.
 
 #include "chrome/browser/media/window_icon_util.h"
 
 #include <X11/Xatom.h>
 #include <X11/Xutil.h>
 
 #include "ui/gfx/x/x11_types.h"
 
+int ErrorHandler(Display* display, XErrorEvent* event) {

[msw, 2016/08/24 17:01:28]

Why is this needed? Can you add an explanatory comment?

[qiangchen, 2016/08/24 18:03:30]

Done.

+  return 0;
+}
+
 gfx::ImageSkia GetWindowIcon(content::DesktopMediaID id) {
   DCHECK(id.type == content::DesktopMediaID::TYPE_WINDOW);
 
   Display* display = gfx::GetXDisplay();
   Atom property = XInternAtom(display, "_NET_WM_ICON", True);
   Atom actual_type;
   int actual_format;
   unsigned long bytes_after;  // NOLINT: type required by XGetWindowProperty
   unsigned long size;
   long* data;
 
+  XSetErrorHandler(&ErrorHandler);
   int status = XGetWindowProperty(display, id.id, property, 0L, ~0L, False,
                                   AnyPropertyType, &actual_type, &actual_format,
                                   &size, &bytes_after,
                                   reinterpret_cast<unsigned char**>(&data));
   if (status != Success) {
     return gfx::ImageSkia();
   }
+  XSetErrorHandler(nullptr);

[msw, 2016/08/24 17:01:28]

Cache the old handler and restore it afterwards, like other callers:
auto old_error_handler = XSetErrorHandler(&ErrorHandler);
...
XSetErrorHandler(old_error_handler);

[qiangchen, 2016/08/24 18:03:30]

Is this quite useful here? The error handler is just a function pointer.
&ErrorHandler is a const. Doing caching is just like store a const in a
variable, and reuse it later.

[msw, 2016/08/24 18:13:56]

My thought is that there may be some pre-existing error handler before you call
XSetErrorHandler, but when you're all done here, you set it to null, instead of
restoring the pre-existing error handler.

[qiangchen, 2016/08/24 18:59:13]

Done.

 
   // The format of |data| is concatenation of sections like
   // [width, height, pixel data of size width * height], and the total bytes
   // number of |data| is |size|. And here we are picking the largest icon.
   int width = 0;
   int height = 0;
   int start = 0;
   int i = 0;
   while (i + 1 < static_cast<int>(size)) {
     if ((i == 0 || static_cast<int>(data[i] * data[i + 1]) > width * height) &&
@@ skipping 15 matching lines @@
   for (long y = 0; y < height; ++y) {
     for (long x = 0; x < width; ++x) {
       pixels_data[result.rowBytesAsPixels() * y + x] =
           static_cast<uint32_t>(data[start + width * y + x]);
     }
   }
 
   XFree(data);
   return gfx::ImageSkia::CreateFrom1xBitmap(result);
 }