Created attachment 216889 [details] [review]
mount-operation: implement org.Gtk.MountOperationHandler

Use the ShellMountOperation dialogs we have to implement a DBus API
allowing other processes to display them.
Since GtkMountOperation now tries to call into our DBus implementation,
every application that uses a GtkMountOperation will gain integration
with our shell dialogs (but will still handle the actual communication
with GVfs).

Review of attachment 216889 [details] [review]:

Mostly fine.

::: js/ui/shellDBus.js
@@ +12,3 @@
 const Flashspot = imports.ui.flashspot;
 const Main = imports.ui.main;
+const ShellMountOperation = imports.ui.shellMountOperation;

?

::: js/ui/shellMountOperation.js
@@ +521,3 @@
+        this._currentType = ShellMountOperationType.NONE;
+        this._currentResponse = Gio.MountOperationResult.UNHANDLED;
+        this._currentResponseDetails = { };

{}

@@ +527,3 @@
+        if (this._currentInvocation) {
+            this._currentInvocation.return_value(
+                GLib.Variant.new('(ua{sv})', [ this._currentResponse, this._currentResponseDetails ]));

Not the biggest fan of the stateful approach, but it works. I'd rather see something like:

    this._returnRequest(invocation, HANDLED, {'password_save': GLib.Variant.new('u', NEVER),
                                              'password': GLib.Variant.new('s': 'hunter2')});

@@ +534,3 @@
+
+    _createGIcon: function(iconName) {
+        let realIconName = (iconName && iconName.length > 0) ? iconName : 'drive-harddisk';

For a string, just iconName ? iconName : 'drive-harddisk' is enough. The empty string evalutes to false.

@@ +560,3 @@
+     */
+    AskPasswordAsync: function(params, invocation) {
+        let [ id, message, iconName, defaultUser, defaultDomain, flags ] = params;

No spaces between [ and ].

@@ +578,3 @@
+        this._dialog.connect('response', Lang.bind(this,
+            function(object, choice, password, remember) {
+                if (choice == '-1') {

This looks smelly.

@@ +702,3 @@
+
+        if (this._dialog) {
+            this._dialog.close(global.get_current_time());

Unnecessary.

Created attachment 216933 [details] [review]
mount-operation: implement org.Gtk.MountOperationHandler

---

Fixed review comments:
- I'd rather keep the stateful approach for now, as it's nice being able to dynamically append properties to the response details variant, and we could need to do it more of it when we start supporting username/domain fields in the future
- also, updated the documentation of the methods to match the new one from bug 674963

Review of attachment 216933 [details] [review]:

Looks fine.

Created attachment 217036 [details] [review]
mount-operation: implement org.Gtk.MountOperationHandler

---

- fixes some problems with multiple dialogs being shown at the same time
- removes the response data from the current state of the handler, as suggested by a previous comment of Jasper
- cleanup the request/response code

Review of attachment 217036 [details] [review]:

I'm not the biggest fan of the half-stateful half-stateless approach, but I guess we don't have any other options.

::: js/ui/shellMountOperation.js
@@ +524,3 @@
+    _sendResponse: function(response, details) {
+        if (this._currentInvocation) {
+            this._currentInvocation.return_value(

Function name is a bit silly, as it could won't send anything if there's no current invocation.

@@ +532,3 @@
+
+    _flushCurrentRequest: function() {
+        this._sendResponse(Gio.MountOperationResult.UNHANDLED, {});

This isn't really a flush anymore. I would just move the sendResponse in the two places it's used.

Created attachment 217044 [details] [review]
mount-operation: implement org.Gtk.MountOperationHandler

---

- removed _flushCurrentRequest()
- renamed _sendResponse() to _clearCurrentRequest()

It's not really half-stateful/half-stateless, just that the state is defined only by an [invocation, id, type] tuple, and the response values are not part of it. I don't really see any way around it, since we need to keep track of those to implement the "update dialog with new values" behavior.

Review of attachment 217044 [details] [review]:

Right. This looks good.

Attachment 217044 [details] pushed as 6b5f9a6 - mount-operation: implement org.Gtk.MountOperationHandler

Thanks, pushed to master.