Created attachment 323720 [details] [review]
Fix PrintOperation object leak

Review of attachment 323720 [details] [review]:

Thanks for the patch, Rafael. Some quick comments:

::: src/photos-base-item.c
@@ +1468,3 @@
+                           G_CALLBACK (photos_base_item_print_operation_done),
+                           self,
+                           G_CONNECT_SWAPPED);

As far as I can tell, this clean up is not related to plugging the leak. Adding the g_clear_object should be enough. So it will be nice if you can split it into a separate patch.

About the clean up itself, I think we can go further. We can do what eog does in eog_window_print. ie., instead of connecting to the signal, just do:
  result = gtk_print_operation_run (...);
  if (result == ...)
    ...

Historical note: Most of the code originated from gnome-documents, but the printing came from eog. The joints between the two code bases shows signs of being welded together in a hurry. gnome-documents uses the "done" signal because EvPrintOperation (from evince) doesn't exactly match the GtkPrintOperation API.

@@ +1478,3 @@
  out:
   g_clear_object (&node);
+  g_clear_object (&print_op);

We should initialize print_op to NULL at the top.

Created attachment 323870 [details] [review]
Fix PrintOperation leak

New version addressing raised points.

Review of attachment 323870 [details] [review]:

Thanks for also adding the error handling. Ideally, this should be split into 3 patches - (i) leak fix (ii) clean up (iii) error handling.

Created attachment 323953 [details] [review]
base-item: Make sure we unref the PrintOperation object

Created attachment 323954 [details] [review]
base-item: Simplify getting the result of running the print operation

Created attachment 323986 [details] [review]
print-setup: Take a reference on the GtkPageSetup

This addresses a CRITICAL seen when cancelling the print dialog without selecting any printer. Eog (eog-print-image-setup.c) doesn't suffer from this because it doesn't unref the member pointers during destruction.

Created attachment 324023 [details] [review]
base-item: Check for errors when printing

Review of attachment 324023 [details] [review]:

::: src/photos-base-item.c
@@ +1465,3 @@
   if (print_res == GTK_PRINT_OPERATION_RESULT_APPLY)
     photos_selection_controller_set_selection_mode (self->priv->sel_cntrlr, FALSE);
+  else if (print_res == GTK_PRINT_OPERATION_RESULT_ERROR && error != NULL)

We only need to check either the print_res or the error. No need to check both.

Created attachment 324067 [details] [review]
base-item: Check for errors when running the print operation

Fixed and pushed. Also tweaked the commit message to add the prefix.