Revert 59147 - Allow overriding of X error functions

BUG=50006 (and various other reports)
TEST=Run chrome under nested window manager using Xephyr (see
http://code.google.com/p/chromium/wiki/LayoutTestsLinux)

use --enable-logging=stderr --log-level=0
kill xephyr
examine log. You should see
X IO Error detected
followed (not necessarily immediately) by
successfully saved /tmp/tx/Default/Preferences
successfully saved /tmp/tx/Local State
successfully saved /tmp/tx/Local State
successfully saved /tmp/tx/Default/Preferences
along with no crash.

There is a high ranking crash report on both linux and chromeos that happens whenever X sends an error to chrome. This change causes us to log and continue when we get a regular error from X. When we get an IO error, indicating X is gone, we attempt to shut down gracefully.

Review URL: http://codereview.chromium.org/3175038

TBR=davemoore@chromium.org
Review URL: http://codereview.chromium.org/3332019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@59175 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 88 insertions, 120 deletions

diff --git a/app/x11_util.cc b/app/x11_util.cc
index 558fd67..ba014dc 100644
--- a/app/x11_util.cc
+++ b/app/x11_util.cc
@@ -20,7 +20,6 @@
 
 #include "base/command_line.h"
 #include "base/logging.h"
-#include "base/stringprintf.h"
 #include "base/thread.h"
 #include "app/x11_util_internal.h"
 #include "gfx/rect.h"
@@ -54,19 +53,26 @@ CachedPictFormats* get_cached_pict_formats() {
 // Maximum number of CachedPictFormats we keep around.
 const size_t kMaxCacheSize = 5;
 
-int DefaultX11ErrorHandler(Display* d, XErrorEvent* e) {
-  LOG(FATAL) << GetErrorEventDescription(e);
+int X11ErrorHandler(Display* d, XErrorEvent* e) {
+  LOG(FATAL) << "X Error detected: serial " << e->serial
+             << " error_code " << static_cast<unsigned int>(e->error_code)
+             << " request_code " << static_cast<unsigned int>(e->request_code)
+             << " minor_code " << static_cast<unsigned int>(e->minor_code);
   return 0;
 }
 
-int DefaultX11IOErrorHandler(Display* d) {
-  // If there's an IO error it likely means the X server has gone away
+int X11IOErrorHandler(Display* d) {
   LOG(FATAL) << "X IO Error detected";
   return 0;
 }
 
 }  // namespace
 
+void SetX11ErrorHandlers() {
+  XSetErrorHandler(X11ErrorHandler);
+  XSetIOErrorHandler(X11IOErrorHandler);
+}
+
 bool XDisplayExists() {
   return (gdk_display_get_default() != NULL);
 }
@@ -455,6 +461,79 @@ void RestackWindow(XID window, XID sibling, bool above) {
   XConfigureWindow(GetXDisplay(), window, CWSibling | CWStackMode, &changes);
 }
 
+XRenderPictFormat* GetRenderVisualFormat(Display* dpy, Visual* visual) {
+  DCHECK(QueryRenderSupport(dpy));
+
+  CachedPictFormats* formats = get_cached_pict_formats();
+
+  for (CachedPictFormats::const_iterator i = formats->begin();
+       i != formats->end(); ++i) {
+    if (i->equals(dpy, visual))
+      return i->format;
+  }
+
+  // Not cached, look up the value.
+  XRenderPictFormat* pictformat = XRenderFindVisualFormat(dpy, visual);
+  CHECK(pictformat) << "XRENDER does not support default visual";
+
+  // And store it in the cache.
+  CachedPictFormat cached_value;
+  cached_value.visual = visual;
+  cached_value.display = dpy;
+  cached_value.format = pictformat;
+  formats->push_front(cached_value);
+
+  if (formats->size() == kMaxCacheSize) {
+    formats->pop_back();
+    // We should really only have at most 2 display/visual combinations:
+    // one for normal browser windows, and possibly another for an argb window
+    // created to display a menu.
+    //
+    // If we get here it's not fatal, we just need to make sure we aren't
+    // always blowing away the cache. If we are, then we should figure out why
+    // and make it bigger.
+    NOTREACHED();
+  }
+
+  return pictformat;
+}
+
+XRenderPictFormat* GetRenderARGB32Format(Display* dpy) {
+  static XRenderPictFormat* pictformat = NULL;
+  if (pictformat)
+    return pictformat;
+
+  // First look for a 32-bit format which ignores the alpha value
+  XRenderPictFormat templ;
+  templ.depth = 32;
+  templ.type = PictTypeDirect;
+  templ.direct.red = 16;
+  templ.direct.green = 8;
+  templ.direct.blue = 0;
+  templ.direct.redMask = 0xff;
+  templ.direct.greenMask = 0xff;
+  templ.direct.blueMask = 0xff;
+  templ.direct.alphaMask = 0;
+
+  static const unsigned long kMask =
+    PictFormatType | PictFormatDepth |
+    PictFormatRed | PictFormatRedMask |
+    PictFormatGreen | PictFormatGreenMask |
+    PictFormatBlue | PictFormatBlueMask |
+    PictFormatAlphaMask;
+
+  pictformat = XRenderFindFormat(dpy, kMask, &templ, 0 /* first result */);
+
+  if (!pictformat) {
+    // Not all X servers support xRGB32 formats. However, the XRENDER spec says
+    // that they must support an ARGB32 format, so we can always return that.
+    pictformat = XRenderFindStandardFormat(dpy, PictStandardARGB32);
+    CHECK(pictformat) << "XRENDER ARGB32 not supported.";
+  }
+
+  return pictformat;
+}
+
 XSharedMemoryId AttachSharedMemory(Display* display, int shared_memory_key) {
   DCHECK(QuerySharedMemorySupport(display));
 
@@ -747,103 +826,4 @@ bool ChangeWindowDesktop(XID window, XID destination) {
   return result == Success;
 }
 
-void SetDefaultX11ErrorHandlers() {
-  SetX11ErrorHandlers(NULL, NULL);
-}
-
-// ----------------------------------------------------------------------------
-// These functions are declared in x11_util_internal.h because they require
-// XLib.h to be included, and it conflicts with many other headers.
-XRenderPictFormat* GetRenderARGB32Format(Display* dpy) {
-  static XRenderPictFormat* pictformat = NULL;
-  if (pictformat)
-    return pictformat;
-
-  // First look for a 32-bit format which ignores the alpha value
-  XRenderPictFormat templ;
-  templ.depth = 32;
-  templ.type = PictTypeDirect;
-  templ.direct.red = 16;
-  templ.direct.green = 8;
-  templ.direct.blue = 0;
-  templ.direct.redMask = 0xff;
-  templ.direct.greenMask = 0xff;
-  templ.direct.blueMask = 0xff;
-  templ.direct.alphaMask = 0;
-
-  static const unsigned long kMask =
-    PictFormatType | PictFormatDepth |
-    PictFormatRed | PictFormatRedMask |
-    PictFormatGreen | PictFormatGreenMask |
-    PictFormatBlue | PictFormatBlueMask |
-    PictFormatAlphaMask;
-
-  pictformat = XRenderFindFormat(dpy, kMask, &templ, 0 /* first result */);
-
-  if (!pictformat) {
-    // Not all X servers support xRGB32 formats. However, the XRENDER spec says
-    // that they must support an ARGB32 format, so we can always return that.
-    pictformat = XRenderFindStandardFormat(dpy, PictStandardARGB32);
-    CHECK(pictformat) << "XRENDER ARGB32 not supported.";
-  }
-
-  return pictformat;
-}
-
-XRenderPictFormat* GetRenderVisualFormat(Display* dpy, Visual* visual) {
-  DCHECK(QueryRenderSupport(dpy));
-
-  CachedPictFormats* formats = get_cached_pict_formats();
-
-  for (CachedPictFormats::const_iterator i = formats->begin();
-       i != formats->end(); ++i) {
-    if (i->equals(dpy, visual))
-      return i->format;
-  }
-
-  // Not cached, look up the value.
-  XRenderPictFormat* pictformat = XRenderFindVisualFormat(dpy, visual);
-  CHECK(pictformat) << "XRENDER does not support default visual";
-
-  // And store it in the cache.
-  CachedPictFormat cached_value;
-  cached_value.visual = visual;
-  cached_value.display = dpy;
-  cached_value.format = pictformat;
-  formats->push_front(cached_value);
-
-  if (formats->size() == kMaxCacheSize) {
-    formats->pop_back();
-    // We should really only have at most 2 display/visual combinations:
-    // one for normal browser windows, and possibly another for an argb window
-    // created to display a menu.
-    //
-    // If we get here it's not fatal, we just need to make sure we aren't
-    // always blowing away the cache. If we are, then we should figure out why
-    // and make it bigger.
-    NOTREACHED();
-  }
-
-  return pictformat;
-}
-
-void SetX11ErrorHandlers(XErrorHandler error_handler,
-                         XIOErrorHandler io_error_handler) {
-  XSetErrorHandler(error_handler ? error_handler : DefaultX11ErrorHandler);
-  XSetIOErrorHandler(
-      io_error_handler ? io_error_handler : DefaultX11IOErrorHandler);
-}
-
-std::string GetErrorEventDescription(XErrorEvent* error_event) {
-  return base::StringPrintf(
-      "X Error detected: %lu error_code %u request_code %u minor_code %u",
-      error_event->serial,
-      error_event->error_code,
-      error_event->request_code,
-      error_event->minor_code);
-}
-// ----------------------------------------------------------------------------
-// End of x11_util_internal.h
-
-
 }  // namespace x11_util
diff --git a/app/x11_util.h b/app/x11_util.h
index c07f42c..8ebed23 100644
--- a/app/x11_util.h
+++ b/app/x11_util.h
@@ -173,10 +173,8 @@ void GrabWindowSnapshot(GtkWindow* gdk_window,
 // Change desktop for |window| to the desktop of |destination| window.
 bool ChangeWindowDesktop(XID window, XID destination);
 
-// Enable the default X error handlers. These will log the error and abort
-// the process if called. Use SetX11ErrorHandlers() from x11_util_internal.h
-// to set your own error handlers.
-void SetDefaultX11ErrorHandlers();
+// Sets the X Error Handlers so we can catch X errors and crash.
+void SetX11ErrorHandlers();
 
 }  // namespace x11_util
 
diff --git a/app/x11_util_internal.h b/app/x11_util_internal.h
index ea3bd3d..9b65dcd 100644
--- a/app/x11_util_internal.h
+++ b/app/x11_util_internal.h
@@ -20,9 +20,9 @@ extern "C" {
 }
 
 namespace x11_util {
-  // --------------------------------------------------------------------------
-  // NOTE: these functions cache the results and must be called from the UI
+  // NOTE: these function caches the results and must be called from the UI
   // thread.
+
   // Get the XRENDER format id for ARGB32 (Skia's format).
   //
   // NOTE:Currently this don't support multiple screens/displays.
@@ -31,16 +31,6 @@ namespace x11_util {
   // Get the XRENDER format id for the default visual on the first screen. This
   // is the format which our GTK window will have.
   XRenderPictFormat* GetRenderVisualFormat(Display* dpy, Visual* visual);
-
-  // --------------------------------------------------------------------------
-  // X11 error handling.
-  // Sets the X Error Handlers. Passing NULL for either will enable the default
-  // error handler, which if called will log the error and abort the process.
-  void SetX11ErrorHandlers(XErrorHandler error_handler,
-                           XIOErrorHandler io_error_handler);
-
-  // Returns a string suitable for logging the error event.
-  std::string GetErrorEventDescription(XErrorEvent* error_event);
 };
 
 #endif  // APP_X11_UTIL_INTERNAL_H_