TouchView Mojom

ash/public/interfaces/touch_view.mojom

Index: ash/public/interfaces/touch_view.mojom
diff --git a/ash/public/interfaces/touch_view.mojom b/ash/public/interfaces/touch_view.mojom
new file mode 100644
index 0000000000000000000000000000000000000000..91d46774cf175aa1037152018ebd2b849f95f918
--- /dev/null
+++ b/ash/public/interfaces/touch_view.mojom
@@ -0,0 +1,16 @@
+// 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.
+
+module ash.mojom;
+
+// Used to listen for when TouchView is toggled.
+interface TouchViewObserver {
+  OnTouchViewChanged(bool enabled);

[James Cook, 2016/12/01 16:55:09]

nit: document when this is fired (e.g. before or after). I presume it's after,
but since mojom is our permanent interface between ash and chrome I prefer
everything to be obsessively documented.

optional: This also could be called OnTouchViewEnabled(bool enabled) or
OnTouchViewToggled(bool enabled) which would make calls like
OnTouchViewChanged(false) self-documenting. Or you could keep separate
Started/Ended methods.

[jonross, 2016/12/01 19:20:57]

Added docs on timing. Going with the suggested OnTouchViewToggled(bool enabled)
I like methods to be as self-documenting as possible.

+};
+
+// Accepts observers which are notified of the current state of TouchView, and
+// of all future state changes.
+interface TouchViewManager {

[James Cook, 2016/12/01 16:55:09]

Just curious, any particular reason for Manager vs. Controller vs. just
TouchView? I'm OK with any of them, and I like that you're calling everything
TouchView, it just seemed a little weird that mojom::TouchViewManager is
implemented by MaximizeModeController.

[jonross, 2016/12/01 19:20:56]

I went with Manager as this interface provides no way to make changes. To me
there was nothing to request from a "Controller". I'm open to suggestions on
this name though, so I could switch it to just TouchView if desired.

Yeah I realized it's weird with MaximizeModeController, but really that should
be renamed at some point to TouchView* as its now does more than simply maximize
windows.

+  AddObserver(TouchViewObserver client);

[James Cook, 2016/12/01 16:55:09]

Just curious, there's no need to remove?

Also, you might move (some of or all of) the interface comment to this method,
just to emphasize that calling the method triggers an initial notification.

[jonross, 2016/12/01 19:20:56]

No need to remove, the mojo::InterfacePtrSet clears our ptrs as connections
close. So when the signin screen is destructed the ash side clears out the
observer.

+};