Reorder browser list on workspace switch

ui/display/desktop_observer.h

+// 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.
+
+#ifndef UI_DISPLAY_DESKTOP_OBSERVER_H_
+#define UI_DISPLAY_DESKTOP_OBSERVER_H_
+
+#include <string>
+
+#include "ui/display/display_export.h"
+
+namespace display {
+
+namespace desktop {

[sky, 2016/06/29 22:55:58]

Don't use desktop here. See threads on namespace usage in chromium-dev for
details.

[danakj, 2016/06/29 22:56:51]

Can you point at a thread? The google style guide says to group things with a
namespace instead of a class, which I learnt recently.

[Tom (Use chromium acct), 2016/06/29 23:24:20]

I'll hold off on making this change until sky/danakj agree on what's best.

[Tom (Use chromium acct), 2016/06/29 23:24:20]

Do you mean to create a display::Desktop class to encapsulate these functions or
just get rid of display::desktop and have them in display?

[sky, 2016/06/30 00:25:55]

Sorry for not being clear. I meant the later, nuke the desktop namespace.

[Tom (Use chromium acct), 2016/06/30 17:35:53]

Done.

+
+// Observers for desktop configuration changes.
+class DISPLAY_EXPORT DesktopObserver {

[sky, 2016/06/29 22:55:58]

Is there a reason you didn't add this to DisplayObserver? X11DesktopHandler can
talk to DesktopScreenX11 to notify appropriately.

[Tom (Use chromium acct), 2016/06/29 23:24:20]

I did that originally, but the desktop::Screen instance is initialized later
during startup than BrowserList, so I was getting segfaults.  I suppose I could
have found where the instance is set, and just added the observer after that,
but I wanted to register the observer in the constructor of BrowserList.

Also (and this may just be nit-picking), I feel like the workspace isn't really
a display or a screen property, but is more of a desktop property.

[sky, 2016/06/30 00:25:55]

Can you move initialization earlier?
I suggested moving workspace onto display/displayobserver as it seems to me the
workspace is logically related to the screen..

[Tom (Use chromium acct), 2016/06/30 17:35:53]

If you really want, I can move it back into DisplayObserver like it was in patch
set 1, and move the Screen initialization earlier

+ public:
+  // Called when the (platform-specific) workspace ID changes to
+  // |new_workspace|.
+  virtual void OnWorkspaceChanged(const std::string& new_workspace) = 0;
+
+ protected:
+  virtual ~DesktopObserver();
+};
+
+DISPLAY_EXPORT void AddObserver(DesktopObserver* observer);
+DISPLAY_EXPORT void RemoveObserver(DesktopObserver* observer);
+
+// Notify all observers that the workspace changed to |new_workspace|.
+DISPLAY_EXPORT void OnWorkspaceChanged(const std::string& new_workspace);
+
+}  // namespace display
+
+}  // namespace desktop
+
+#endif  // UI_DISPLAY_DESKTOP_OBSERVER_H_