Have panels respond to changes in work area on Linux.

ui/base/x/active_window_watcher_x.h

 // Copyright (c) 2011 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 UI_BASE_X_ACTIVE_WINDOW_WATCHER_X_H_
 #define UI_BASE_X_ACTIVE_WINDOW_WATCHER_X_H_
 #pragma once
 
 #include <gdk/gdk.h>
 
-#include "base/basictypes.h"
 #include "base/memory/singleton.h"
 #include "base/observer_list.h"
-#include "ui/base/gtk/gtk_signal.h"
 #include "ui/base/ui_export.h"
+#include "ui/base/x/x11_util.h"
 
 namespace ui {
 
 // This is a helper class that is used to keep track of which window the X
-// window manager thinks is active. Add an Observer to listener for changes to
+// window manager thinks is active. Add an Observer to listen for changes to
 // the active window.
 class UI_EXPORT ActiveWindowWatcherX {
  public:
   class Observer {
    public:
     // |active_window| will be NULL if the active window isn't one of Chrome's.
     virtual void ActiveWindowChanged(GdkWindow* active_window) = 0;
 
    protected:
     virtual ~Observer() {}
   };
 
   static ActiveWindowWatcherX* GetInstance();
-
   static void AddObserver(Observer* observer);
   static void RemoveObserver(Observer* observer);
+  static Atom GetPropertyAtom();

[sky, 2011/11/18 17:54:22]

Add a description.

[prasadt, 2011/11/18 22:31:42]

Done.

+
+  // Sends a notification out through the NotificationService that the active
+  // window has changed.
+  static void Notify();

[sky, 2011/11/18 17:54:22]

This doesn't notify through the NOtificationService.

I don't like exposing this as public as folks shouldn't be using it directly.
Maybe RootWindowPropertyWatcherX should notify via an interface that this class
(and WorkAreaWatcherX) implement. That way RootWindowPropertyWatcherX doesn't
need to know about this class (or WorkAreaWatcherX).

[prasadt, 2011/11/18 22:31:42]

I did consider that option. Didn't do it for a few reasons:
1) That would entail ActiveWindowWatcherX and WorkAreaWatcherX classes
registering with RootWindowPropertyWatcherX to be notified for their specific
properties and RootWindowPropertyWatcherX maintaining a map to associate each
property with a specific watcher. Seemed unnecessarily heavy weight to me
considering we're only interested in two properties at this time.
2) If I defined that interface, it'd be strange not to add AddObserver and
RemoveObserver methods to that interface. That'd make them instance methods and
will mean changing the call sites for ActiveWindowWatcherX.
3) And adding this interface won't really make Notify() private any way, as
RootWindowPropertyWatcherX will still need to be able to call it.

The other option I considered is making RootWindowPropertyWatcherX a friend of
ActiveWindowWatcherX and WorkAreaWatcherX. That doesn't solve the issue of
RootWindowPropertyWatcherX knowing about these other classes but will keep
Notify private. I just avoided it on principle but can do that if Notify being
public is the big issue.

I settled on what I got right now because its simple and adequate for the
current requirements. Notify() even though not desirable to be public, its not
private either as RootWindowPropertyWatcherX needs to be able to call it.

Let me know what you think.

[sky, 2011/11/18 22:44:58]

I'm ok with it as long as you make RootWindowPropertyWatcherX a friend of these
classes so Notify can be private.

[prasadt, 2011/11/21 23:36:49]

Done.

 
   // Checks if the WM supports the active window property. Note that the return
   // value can change, especially during system startup.
   static bool WMSupportsActivation();
 
  private:
   friend struct DefaultSingletonTraits<ActiveWindowWatcherX>;
 
-  ActiveWindowWatcherX();
-  ~ActiveWindowWatcherX();
-
-  void Init();
-
-  // Sends a notification out through the NotificationService that the active
-  // window has changed.
+  ActiveWindowWatcherX() {}

[sky, 2011/11/18 17:54:22]

Don't inline these.

[prasadt, 2011/11/18 22:31:42]

Done.

Curious though why this is discouraged.

[sky, 2011/11/18 22:44:58]

See threads from erg on this. Your class is probably fine since it doesn't have
much state, but there is nothing worse than landing a change a breaking a clang
bot because some change caused the threshold to be exceeded.

[sky, 2011/11/18 22:46:39]

Ha. I was just asked to review a change that caused such bustage. See
http://codereview.chromium.org/8558010/ . 

+  ~ActiveWindowWatcherX() {}
   void NotifyActiveWindowChanged();
 
-  // Callback for PropertyChange XEvents.
-  CHROMEG_CALLBACK_1(ActiveWindowWatcherX, GdkFilterReturn,
-                     OnWindowXEvent, GdkXEvent*, GdkEvent*);
-
   ObserverList<Observer> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(ActiveWindowWatcherX);
 };
 
 }  // namespace ui
 
 #endif  // UI_BASE_X_ACTIVE_WINDOW_WATCHER_X_H_