Move ui/base/x/x11_error_tracker to base/x11.

Move ui/base/x/x11_error_tracker to base/x11.

This is to share common x11 code for ui/x and chromeos/display.

R=thakis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225662

[kcwu, 2013-09-23 11:40:11]

Code from chromeos/display (https://codereview.chromium.org/24039002/, under
review) wants to use ui/base/x/x11_error_tracker.
So I move it to base/x11.

[Daniel Erat, 2013-09-23 14:42:09]

https://codereview.chromium.org/24160005/diff/1/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/24160005/diff/1/base/base.gyp#newcode79
base/base.gyp:79: }, {  # use_11==0
nit: use_x11

but do you even need this? it looks like this should already be excluded on
non-x11 systems by build/filename_rules.gypi.

[kcwu, 2013-09-23 15:46:08]

https://codereview.chromium.org/24160005/diff/1/base/base.gyp
File base/base.gyp (right):

https://codereview.chromium.org/24160005/diff/1/base/base.gyp#newcode79
base/base.gyp:79: }, {  # use_11==0
On 2013/09/23 14:42:09, Daniel Erat wrote:
> nit: use_x11
> 
> but do you even need this? it looks like this should already be excluded on
> non-x11 systems by build/filename_rules.gypi.

Done.
Thanks. I didn't know these rules.

[Daniel Erat, 2013-09-23 15:49:23]

Thanks, LGTM (but you still need Nico's approval).

[Nico, 2013-09-23 16:38:18]

I didn't look at the linked to review, but base feels like a strange place for
this to me.

base sits below UI widget toolkits for the most part*. base-type functionality
for ui widget toolkits is in ui/base (where this code was), which feels like a
better place. If the code that needs this class needs basic helper classes for
ui widget toolkits, shouldn't it depend on ui/base instead?

Adding a base/x11 folder looks strange to me.


*: The message loops need to know a bit about UI toolkit stuff. That decision
predates me, but I'm guessing the reasoning is that message loops are very
in-scope for base and they need to cooperate with UI pumps to some degree.

[Elliot Glaysher, 2013-09-23 22:34:06]

+ brettw who might have an opinion, since he signed off on adding a base/x11/
directory in https://codereview.chromium.org/23536057/

After working through the dependencies, we can't put code shared between
chromeos/ and ui/ because ui/ depends on chromeos/ during tests (and in general,
but this doesn't look like it's explicitly stated and is resolved with the
linking of the main binaries). See the including of the
'chromeos_test_support_wihtout_gmock' target in ui_unittests.gypi. This looks to
mostly be about IME related code.

I agree that the state of the world is bad. I have no idea how to fix the above
circular dependency. If we want to share code between chromeos/ and ui/, it has
to go somewhere else. base/ is the most logical choice.

[brettw, 2013-09-24 23:22:31]

lgtm

[Elliot Glaysher, 2013-09-24 23:53:02]

lgtm

[commit-bot: I haz the power, 2013-09-25 01:33:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kcwu@chromium.org/24160005/1012

[commit-bot: I haz the power, 2013-09-25 01:33:06]

Failed to apply patch for ui/base/x/x11_util.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/base/x/x11_util.cc
  Hunk #3 succeeded at 959 (offset 7 lines).
  Hunk #4 FAILED at 977.
  Hunk #5 succeeded at 1252 (offset -137 lines).
  1 out of 5 hunks FAILED -- saving rejects to file ui/base/x/x11_util.cc.rej

Patch:       ui/base/x/x11_util.cc
Index: ui/base/x/x11_util.cc
diff --git a/ui/base/x/x11_util.cc b/ui/base/x/x11_util.cc
index
58c2c698a3cf825780d1c41ba5ce88bd7972b90c..93538461784fb95480bdfd755a2ef7d78a70a21f
100644
--- a/ui/base/x/x11_util.cc
+++ b/ui/base/x/x11_util.cc
@@ -32,11 +32,11 @@
 #include "base/strings/stringprintf.h"
 #include "base/sys_byteorder.h"
 #include "base/threading/thread.h"
+#include "base/x11/x11_error_tracker.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkPostConfig.h"
 #include "ui/base/touch/touch_factory_x11.h"
 #include "ui/base/x/device_data_manager.h"
-#include "ui/base/x/x11_error_tracker.h"
 #include "ui/base/x/x11_util_internal.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/keycodes/keyboard_code_conversion_x.h"
@@ -365,7 +365,7 @@ static SharedMemorySupport
DoQuerySharedMemorySupport(XDisplay* dpy) {
   memset(&shminfo, 0, sizeof(shminfo));
   shminfo.shmid = shmkey;
 
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   bool result = XShmAttach(dpy, &shminfo);
   if (result)
     VLOG(1) << "X got shared memory segment " << shmkey;
@@ -952,7 +952,7 @@ bool SetIntArrayProperty(XID window,
   for (size_t i = 0; i < value.size(); ++i)
     data[i] = value[i];
 
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   XChangeProperty(gfx::GetXDisplay(),
                   window,
                   name_atom,
@@ -977,7 +977,7 @@ bool SetAtomArrayProperty(XID window,
   for (size_t i = 0; i < value.size(); ++i)
     data[i] = value[i];
 
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   XChangeProperty(gfx::GetXDisplay(),
                   window,
                   name_atom,
@@ -1389,7 +1389,7 @@ bool GetWindowManagerName(std::string* wm_name) {
   // _NET_SUPPORTING_WM_CHECK property pointing to itself (to avoid a stale
   // property referencing an ID that's been recycled for another window), so we
   // check that too.
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   int wm_window_property = 0;
   bool result = GetIntProperty(
       wm_window, "_NET_SUPPORTING_WM_CHECK", &wm_window_property);

[commit-bot: I haz the power, 2013-09-25 03:08:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kcwu@chromium.org/24160005/12001

[commit-bot: I haz the power, 2013-09-25 04:24:25]

Retried try job too often on linux_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-09-25 07:24:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kcwu@chromium.org/24160005/12001

[commit-bot: I haz the power, 2013-09-25 08:27:55]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[kcwu, 2013-09-25 09:06:13]

I found I still need approval for 	
chrome/browser/screensaver_window_finder_x11.cc

thakis@, do you approve?

[Nico, 2013-09-26 17:32:18]

lgtm

[commit-bot: I haz the power, 2013-09-27 01:02:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kcwu@chromium.org/24160005/12001

[commit-bot: I haz the power, 2013-09-27 01:02:07]

Failed to apply patch for ui/base/x/x11_util.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/base/x/x11_util.cc
  Hunk #1 FAILED at 32.
  Hunk #2 succeeded at 302 (offset -63 lines).
  Hunk #3 succeeded at 871 (offset -63 lines).
  Hunk #4 succeeded at 896 (offset -63 lines).
  Hunk #5 succeeded at 1189 (offset -63 lines).
  1 out of 5 hunks FAILED -- saving rejects to file ui/base/x/x11_util.cc.rej

Patch:       ui/base/x/x11_util.cc
Index: ui/base/x/x11_util.cc
diff --git a/ui/base/x/x11_util.cc b/ui/base/x/x11_util.cc
index
accd4f975b9d67e3f83abcffcad80ca605a332f4..2fe355d3daaed1fe855c003579dd27bad6e16e25
100644
--- a/ui/base/x/x11_util.cc
+++ b/ui/base/x/x11_util.cc
@@ -32,11 +32,11 @@
 #include "base/strings/stringprintf.h"
 #include "base/sys_byteorder.h"
 #include "base/threading/thread.h"
+#include "base/x11/x11_error_tracker.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkPostConfig.h"
 #include "ui/base/touch/touch_factory_x11.h"
 #include "ui/base/x/device_data_manager.h"
-#include "ui/base/x/x11_error_tracker.h"
 #include "ui/base/x/x11_util_internal.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/keycodes/keyboard_code_conversion_x.h"
@@ -365,7 +365,7 @@ static SharedMemorySupport
DoQuerySharedMemorySupport(XDisplay* dpy) {
   memset(&shminfo, 0, sizeof(shminfo));
   shminfo.shmid = shmkey;
 
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   bool result = XShmAttach(dpy, &shminfo);
   if (result)
     VLOG(1) << "X got shared memory segment " << shmkey;
@@ -934,7 +934,7 @@ bool SetIntArrayProperty(XID window,
   for (size_t i = 0; i < value.size(); ++i)
     data[i] = value[i];
 
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   XChangeProperty(gfx::GetXDisplay(),
                   window,
                   name_atom,
@@ -959,7 +959,7 @@ bool SetAtomArrayProperty(XID window,
   for (size_t i = 0; i < value.size(); ++i)
     data[i] = value[i];
 
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   XChangeProperty(gfx::GetXDisplay(),
                   window,
                   name_atom,
@@ -1252,7 +1252,7 @@ bool GetWindowManagerName(std::string* wm_name) {
   // _NET_SUPPORTING_WM_CHECK property pointing to itself (to avoid a stale
   // property referencing an ID that's been recycled for another window), so we
   // check that too.
-  X11ErrorTracker err_tracker;
+  base::X11ErrorTracker err_tracker;
   int wm_window_property = 0;
   bool result = GetIntProperty(
       wm_window, "_NET_SUPPORTING_WM_CHECK", &wm_window_property);

[commit-bot: I haz the power, 2013-09-27 01:49:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kcwu@chromium.org/24160005/35001

[commit-bot: I haz the power, 2013-09-27 05:07:43]

Change committed as 225662