Created attachment 206201 [details] [review]
ephy-download: emit completed after processing download

Allowing the user to act freely on the EphyDownload object when
the completed signal is called, i.e: unrefing the object.

Created attachment 206202 [details] [review]
tests: ephy-download: missing ephy_download_start

Created attachment 206207 [details] [review]
ephy-download: emit completed after processing download

Allowing the user to act freely on the EphyDownload object when
the completed signal is called, i.e: unrefing the object.

Also use a switch instead of lots of ifs.

==

This is an alternative to the previos patch, plaes correctly suggested to use a switch instead of an if for this.

Review of attachment 206207 [details] [review]:

Does this fix anything? FWIW, if you perform all the actions before "completed" is emitted it's not very clear what can you do in the callback.

::: embed/ephy-download.c
@@ +902,3 @@
+      } else {
+        ephy_download_do_download_action (download, EPHY_DOWNLOAD_ACTION_NONE);
+      }

While you are at it, no braces in single line clauses.

Review of attachment 206202 [details] [review]:

OK, minus the HTML_STRING thing.

::: tests/ephy-download.c
@@ +35,3 @@
 #include <string.h>
 
+#define HTML_STRING "testing-ephy-download"

Unrelated? Make this another commit (and just go ahead and commit it).

(In reply to comment #4)
> Review of attachment 206207 [details] [review]:
> 
> Does this fix anything? FWIW, if you perform all the actions before "completed"
> is emitted it's not very clear what can you do in the callback.
> 

I discovered that in the test I couldn't unref the EphyDownload object in the completed callback. ephy_download_do_download_action received a NULL @download when I did that.

I considered that the callback should allow you to kill the @download object.
Another option is to hold a ref and free it on ephy_download_do_download_action?

Consider that the signal is not meant to allow you to stop the default handler like other API. We could change it to do so however, and returning TRUE would mean to not call ephy_download_do_download_action.

(In reply to comment #6)
> I considered that the callback should allow you to kill the @download object.
> Another option is to hold a ref and free it on
> ephy_download_do_download_action?

Hrm. I'd say ephy itself should get rid of the object if it can? For instance, connecting to "completed" just to unref the object seems pretty pointless.

> 
> Consider that the signal is not meant to allow you to stop the default handler
> like other API. We could change it to do so however, and returning TRUE would
> mean to not call ephy_download_do_download_action.

I guess we should not do that until we actually need it for some reason.

Created attachment 206468 [details] [review]
tests: ephy-download: missing ephy_download_start

This version doesn't require the API adjust.

Review of attachment 206468 [details] [review]:

OK.

Attachment 206468 [details] pushed as 87b9227 - tests: ephy-download: missing ephy_download_start

Done.