Created attachment 240189 [details] [review]
Allow updating extensions

Please use the egowrapper.py here. I want to keep this conceptually separate to the rest of the code in case the web api changes, etc.

https://github.com/nzjrs/gnome-tweak-tool/blob/ego/gtweak/egowrapper.py

Review of attachment 240189 [details] [review]:

sorry

Created attachment 240277 [details] [review]
Allow updating extensions

Review of attachment 240277 [details] [review]:

Please import egowrapper as it was in the ego branch. Add a second parameter (the uuid) to the got-extension-info signal.

create one ego instance in the ShellExtensionTweakGroup object (so that the number of simultaneous connections to e.g.o is limited by the SoupSessionAsync object. We don't want to denial-of-service the website). Then do something like the following (in each tweak, or in the tweak group)

egowrapper.connect("got-extension-info" self._got_info)

def got_info(self, uuid, info):
    if uuid == self.this_tweak_uuid:
       if there is some upgrade by looking at info:
           self._add_upgrade_button()

::: gtweak/egowrapper.py
@@ +34,3 @@
+        self._extension = extension
+        self._widget = widget
+        self._button = button

Buttons and ui objects should not belong here, use something closer to the egowrapper in my branch.

::: gtweak/tweaks/tweak_shell_extensions.py
@@ +112,3 @@
+        stream = Gio.File.new_for_uri(url)
+        stream.load_contents_async(None, self.download_file_complete, uuid)
+        self.widget.remove(btn)

Shouldn't you just call the update dbus method on the gnome shell object?

@@ +116,3 @@
+    def download_file_complete(self, extension_file, res, uuid):
+        content = extension_file.load_contents_finish(res)[1]
+        stream = Gio.File.new_for_uri(url)

This is the sort of code I wan't left in the egowrapper object. Just use my copy from the ego branch.

@@ +154,3 @@
+            f = chooser
+        else:
+            f = chooser.get_filename()

Don't know why this is here

@@ +214,3 @@
         #set button back to default state
+        if type(chooser) != str:
+            chooser.unselect_all()

Don't know why this is here

Created attachment 240918 [details] [review]
Allow updating extensions

Review of attachment 240918 [details] [review]:

Almost there, great work.

::: gtweak/tweaks/tweak_shell_extensions.py
@@ +108,3 @@
+        self.widget.set_sensitive(False)
+        thread = threading.Thread(target=self.download_extension, args=(btn,uuid,))
+        thread.start()

Hmm, it is annoying that you need to use a thread here.... I presume this is because install_remote_extension blocks?

@@ +114,3 @@
+        if status == 's':
+            self.widget.remove(btn)
+        thread.start()

This is being run from the non_main thread, and is unsafe. I'm surprised it didnt crash here. GObject.threads_init() doesnt prevent this.

You should do this

if status == 's':
    GObject.idle_add(lambda x: s.set_sensitive(True), widget)

that way the Gtk call is made in the gtk thread.

Don't bother removing the button, just change the sensitivity.

@@ +122,3 @@
+        self.widget.reorder_child(button, 1)
+        if self.widget.get_visible() == True:
+        if status == 's':

OK, although you should add a comment saying why you need to test visibility here (in case the user has gone to a different section?)

(In reply to comment #7)
> Review of attachment 240918 [details] [review]:
> 
> Almost there, great work.
> 
> ::: gtweak/tweaks/tweak_shell_extensions.py
> @@ +108,3 @@
> +        self.widget.set_sensitive(False)
> +        thread = threading.Thread(target=self.download_extension,
> args=(btn,uuid,))
> +        thread.start()
> 
> Hmm, it is annoying that you need to use a thread here.... I presume this is
> because install_remote_extension blocks?

Yes, when I call to the install remote extension directly from the main thread, the UI freezes while downloads the extension.

> @@ +114,3 @@
> +        if status == 's':
> +            self.widget.remove(btn)
> +        thread.start()
> 
> This is being run from the non_main thread, and is unsafe. I'm surprised it
> didnt crash here. GObject.threads_init() doesnt prevent this.
> 
> You should do this
> 
> if status == 's':
>     GObject.idle_add(lambda x: s.set_sensitive(True), widget)
> 
> that way the Gtk call is made in the gtk thread.
> 
> Don't bother removing the button, just change the sensitivity.
> 
> @@ +122,3 @@
> +        self.widget.reorder_child(button, 1)
> +        if self.widget.get_visible() == True:
> +        if status == 's':
> 
> OK, although you should add a comment saying why you need to test visibility
> here (in case the user has gone to a different section?)

Created attachment 241114 [details] [review]
Allow updating extensions

Attachment 241114 [details] pushed as 08f8cda - Allow updating extensions

The use of threading here is entirely unnecessary, and probably harmful - http://dbus.freedesktop.org/doc/dbus-python/doc/tutorial.html describes passing reply_handler in when calling a D-Bus method to make it async.

Reopening for Alex to fix add per Owens suggestion

Alex, ping?

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.