Created attachment 267735 [details] [review]
patch which adds the right scope

Review of attachment 267735 [details] [review]:

As with the patch in https://bugzilla.gnome.org/show_bug.cgi?id=723091 you should also add a new capability, so that printing can be toggled off.

Review of attachment 267735 [details] [review]:

This is really cool! I am worried about one thing, though. If gtk+ depends on gnome-online-accounts, which already depends on gtk+, won't it create a circular dependency?

Apart from what Bastien said ...

::: src/goabackend/goagoogleprovider.c
@@ +147,1 @@
          /* Google Talk */

Also, bump the credentials generation so that OAuth 2.0 tokens obtained with the old set of scopes are refreshed with the new set.

Created attachment 267746 [details] [review]
gnome-online-accounts-cloudprint.patch

Is this better?

As an aside:
Now that I look closer at gnome-online-accounts, it looks like it would be possible to move the actual code that enumerates printers, gets printer details, and submits print jobs into the goa provider.

That sort of chimes with some thoughts I've been having about how to unify different types of "cloud printing":
http://cyberelk.net/tim/2012/03/08/session-printing/

Review of attachment 267746 [details] [review]:

Thanks for the patch. This looks good apart from some minor issues.

::: data/dbus-interfaces.xml
@@ +166,3 @@
     <property name="DocumentsDisabled" type="b" access="readwrite"/>
 
+    <!-- PrintersDisabled:

@since: 3.12.0

@@ +672,3 @@
 
+  <!--
+      org.gnome.OnlineAccounts.Printers:

@since: 3.12.0

::: src/goabackend/goagoogleprovider.c
@@ +144,3 @@
 
+	 /* Google Cloud Print */
+	 "https://www.googleapis.com/auth/cloudprint "

Where is this scope documented? I tried looking at various places under https://developers.google.com/cloud-print/docs/overview but could not find it.

Could you please add the URL where the scope is documented to README? We deal with quite a large number of providers and services and it helps to have the relevant URLs at hand.

(In reply to comment #4)
> As an aside:
> Now that I look closer at gnome-online-accounts, it looks like it would be
> possible to move the actual code that enumerates printers, gets printer
> details, and submits print jobs into the goa provider.

It would be possible but I do not want to do that. I do not want to have lots of "domain-specific" code in GOA. eg., it is also tempting have full blown IMAP and SMTP support in GOA so that MUAs do not need to have their own.

I want to limit GOA to being a "dumb" account store, that exports just enough information that:
 - a user would have to otherwise manually enter into an application
 - is needed to uniquely identify an account

eg., the username, provider branding, email server address:port, WebDAV URLs, etc..

... leaving the "domain-specific" stuff to the particular libraries and applications. eg., libgdata implements a bunch of the Google APIs and you can optionally use GOA with it, similarly libzapojit for Windows Live, evolution/e-d-s for mail/calendaring/contacts, gvfs for WebDAV, and now gtk+ for printing.

Created attachment 267969 [details] [review]
patch with fixes from comments

Yes, OK.

Here's a patch with the suggested improvements.

Actually this feature is aimed at 3.11 so I guess it should read "@since: 3.11.0"...
(https://wiki.gnome.org/ThreePointEleven/Features/CloudPrinting)

(In reply to comment #8)
> Actually this feature is aimed at 3.11 so I guess it should read "@since:
> 3.11.0"...
> (https://wiki.gnome.org/ThreePointEleven/Features/CloudPrinting)

"Since" means "First stable version containing the API". Not when it appeared in devel versions.

Thanks, got it.

Review of attachment 267746 [details] [review]:

::: src/goabackend/goagoogleprovider.c
@@ +477,3 @@
+        {
+          printers = goa_printers_skeleton_new ();
+          goa_object_skeleton_set_documents (object, printers);

Typo alert! Should be goa_object_skeleton_set_printers, and not _set_documents.

Review of attachment 267969 [details] [review]:

Looks good, apart from the minor typo that I forgot to point out before.

Created attachment 267976 [details] [review]
Added Printers capability and use it in google backend

Fixed the typo.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.

*** Bug 726374 has been marked as a duplicate of this bug. ***