Merge remote-tracking branch 'origin/test/12059-dogtail-optimizations' into stable (Fix-committed: #12059).

7 files changed, 134 insertions, 144 deletions

diff --git a/features/scripts/vm-execute b/features/scripts/vm-execute
index 6486a2e..06ff5a2 100755
--- a/features/scripts/vm-execute
+++ b/features/scripts/vm-execute
@@ -46,7 +46,7 @@ opt_parser = OptionParser.new do |opts|
 end
 opt_parser.parse!(ARGV)
 cmd = ARGV.join(" ")
-c = RemoteShell::Command.new(FakeVM.new, cmd, cmd_opts)
+c = RemoteShell::ShellCommand.new(FakeVM.new, cmd, cmd_opts)
 puts "Return status: #{c.returncode}"
 puts "STDOUT:\n#{c.stdout}"
 puts "STDERR:\n#{c.stderr}"
diff --git a/features/step_definitions/browser.rb b/features/step_definitions/browser.rb
index 97618a7..8334dc0 100644
--- a/features/step_definitions/browser.rb
+++ b/features/step_definitions/browser.rb
@@ -130,12 +130,12 @@ Then /^"([^"]+)" has loaded in the Tor Browser$/ do |title|
   end
   expected_title = "#{title} - #{browser_name}"
   app = Dogtail::Application.new('Firefox')
-  app.child(expected_title, roleName: 'frame').wait(60)
+  try_for(60) { app.child(expected_title, roleName: 'frame') }
   # The 'Reload current page' button (graphically shown as a looping
   # arrow) is only shown when a page has loaded, so once we see the
   # expected title *and* this button has appeared, then we can be sure
   # that the page has fully loaded.
-  app.child(reload_action, roleName: 'push button').wait(60)
+  try_for(60) { app.child(reload_action, roleName: 'push button') }
 end
 
 Then /^the (.*) has no plugins installed$/ do |browser|
@@ -235,9 +235,6 @@ end
 Then /^the Tor Browser shows the "([^"]+)" error$/ do |error|
   firefox = Dogtail::Application.new('Firefox')
   page = firefox.child("Problem loading page", roleName: "document frame")
-  # Important to wait here since children() won't retry but return the
-  # immediate results
-  page.wait
   headers = page.children(roleName: "heading")
   found = headers.any? { |heading| heading.text == error }
   raise "Could not find the '#{error}' error in the Tor Browser" unless found
diff --git a/features/step_definitions/common_steps.rb b/features/step_definitions/common_steps.rb
index d48a01b..643af7b 100644
--- a/features/step_definitions/common_steps.rb
+++ b/features/step_definitions/common_steps.rb
@@ -720,7 +720,7 @@ end
 Given /^I start "([^"]+)" via the GNOME "([^"]+)" applications menu$/ do |app_name, submenu|
   app = Dogtail::Application.new('gnome-shell')
   for element in ['Applications', submenu, app_name] do
-    app.child(element, roleName: 'label').click
+    app.child(element, roleName: 'label', showingOnly: true).click
   end
 end
 
diff --git a/features/step_definitions/icedove.rb b/features/step_definitions/icedove.rb
index 07625c0..f5b3fb3 100644
--- a/features/step_definitions/icedove.rb
+++ b/features/step_definitions/icedove.rb
@@ -32,7 +32,7 @@ When /^I start Icedove$/ do
     $vm.file_append('/etc/icedove/pref/icedove.js ', line)
   end
   step 'I start "Icedove" via the GNOME "Internet" applications menu'
-  icedove_main.wait(60)
+  try_for(60) { icedove_main }
 end
 
 When /^I have not configured an email account$/ do
@@ -44,7 +44,7 @@ When /^I have not configured an email account$/ do
 end
 
 Then /^I am prompted to setup an email account$/ do
-  icedove_wizard.wait(30)
+  icedove_wizard
 end
 
 Then /^I cancel setting up an email account$/ do
@@ -57,7 +57,6 @@ Then /^I open Icedove's Add-ons Manager$/ do
   @icedove_addons = icedove_app.child(
     'Add-ons Manager - Icedove Mail/News', roleName: 'frame'
   )
-  @icedove_addons.wait
 end
 
 Then /^I click the extensions tab$/ do
@@ -86,7 +85,14 @@ When /^I go into Enigmail's preferences$/ do
 end
 
 When /^I enable Enigmail's expert settings$/ do
-  @enigmail_prefs.button('Display Expert Settings and Menus').click
+  # Clicking the "Display..." button sometimes fails, presumably
+  # because the GUI hasn't loaded completely (or perhaps the button
+  # gets its action connected *after* the button is displayed?), so we
+  # have to verify that the click actually happened.
+  retry_action(5) do
+    @enigmail_prefs.button('Display Expert Settings and Menus').click
+    @enigmail_prefs.button('Hide Expert Settings and Menus')
+  end
 end
 
 Then /^I click Enigmail's (.+) tab$/ do |tab_name|
@@ -111,7 +117,7 @@ end
 
 Then /^I see that Torbirdy is configured to use Tor$/ do
   icedove_main.child(roleName: 'status bar')
-    .child('TorBirdy Enabled:    Tor', roleName: 'label').wait
+    .child('TorBirdy Enabled:    Tor', roleName: 'label')
 end
 
 When /^I enter my email credentials into the autoconfiguration wizard$/ do
@@ -121,7 +127,7 @@ When /^I enter my email credentials into the autoconfiguration wizard$/ do
     .typeText($config['Icedove']['password'])
   icedove_wizard.button('Continue').click
   # This button is shown if and only if a configuration has been found
-  icedove_wizard.button('Done').wait(120)
+  try_for(120) { icedove_wizard.button('Done') }
 end
 
 Then /^the autoconfiguration wizard's choice for the (incoming|outgoing) server is secure (.+)$/ do |type, protocol|
@@ -140,24 +146,31 @@ When /^I fetch my email$/ do
 
   icedove_main.child('Mail Toolbar', roleName: 'tool bar')
     .button('Get Messages').click
-  fetch_progress = icedove_main.child(roleName: 'status bar')
-                     .child(roleName: 'progress bar')
-  fetch_progress.wait_vanish(120)
+  try_for(120) do
+    begin
+      icedove_main.child(roleName: 'status bar', retry: false)
+        .child(roleName: 'progress bar', retry: false)
+      false
+    rescue
+      true
+    end
+  end
 end
 
 When /^I accept the (?:autoconfiguration wizard's|manual) configuration$/ do
   # The password check can fail due to bad Tor circuits.
   retry_tor do
     try_for(120) do
-      if icedove_wizard.exist?
+      begin
         # Spam the button, even if it is disabled (while it is still
         # testing the password).
         icedove_wizard.button('Done').click
         false
-      else
+      rescue
         true
       end
     end
+    true
   end
   # The account isn't fully created before we fetch our mail. For
   # instance, if we'd try to send an email before this, yet another
@@ -196,7 +209,6 @@ end
 When /^I send an email to myself$/ do
   icedove_main.child('Mail Toolbar', roleName: 'tool bar').button('Write').click
   compose_window = icedove_app.child('Write: (no subject)')
-  compose_window.wait(10)
   compose_window.child('To:', roleName: 'autocomplete').child(roleName: 'entry')
     .typeText($config['Icedove']['address'])
   # The randomness of the subject will make it easier for us to later
@@ -210,7 +222,9 @@ When /^I send an email to myself$/ do
     .typeText('test')
   compose_window.child('Composition Toolbar', roleName: 'tool bar')
     .button('Send').click
-  compose_window.wait_vanish(120)
+  try_for(120) do
+    not compose_window.exist?
+  end
 end
 
 Then /^I can find the email I sent to myself in my inbox$/ do
@@ -221,7 +235,6 @@ Then /^I can find the email I sent to myself in my inbox$/ do
                                 roleName: 'entry')
     filter.typeText(@subject)
     hit_counter = icedove_main.child('1 message')
-    hit_counter.wait
     inbox_view = hit_counter.parent
     message_list = inbox_view.child(roleName: 'table')
     the_message = message_list.children(roleName: 'table row').find do |message|
diff --git a/features/support/helpers/dogtail.rb b/features/support/helpers/dogtail.rb
index 2d81205..21ddcf2 100644
--- a/features/support/helpers/dogtail.rb
+++ b/features/support/helpers/dogtail.rb
@@ -45,59 +45,51 @@ module Dogtail
   #     menu.click()
   #
   # i.e. the object referenced by `menu` is never modified by method
-  # calls and can be used as expected. This explains why
-  # `proxy_call()` below returns a new instance instead of adding
-  # appending the new component the proxied method call would result
-  # in.
+  # calls and can be used as expected.
 
   class Application
+    @@node_counter ||= 0
 
     def initialize(app_name, opts = {})
+      @var = "node#{@@node_counter += 1}"
       @app_name = app_name
       @opts = opts
-      @init_lines = @opts[:init_lines] || [
-        "from dogtail import tree",
-        "from dogtail.config import config",
-        "config.logDebugToFile = False",
-        "config.logDebugToStdOut = False",
-        "config.blinkOnActions = True",
-        "config.searchShowingOnly = True",
-        "application = tree.root.application('#{@app_name}')",
+      @opts[:user] ||= LIVE_USER
+      @find_code = "dogtail.tree.root.application('#{@app_name}')"
+      script_lines = [
+        "import dogtail.config",
+        "import dogtail.tree",
+        "import dogtail.predicate",
+        "dogtail.config.logDebugToFile = False",
+        "dogtail.config.logDebugToStdOut = False",
+        "dogtail.config.blinkOnActions = True",
+        "dogtail.config.searchShowingOnly = True",
+        "#{@var} = #{@find_code}",
       ]
-      @components = @opts[:components] || ['application']
+      run(script_lines)
     end
 
-    def build_script(lines)
-      (
-        ["#!/usr/bin/python"] +
-        @init_lines +
-        lines
-      ).join("\n")
+    def to_s
+      @var
     end
 
-    def build_line
-      @components.join('.')
+    def run(code)
+      code = code.join("\n") if code.class == Array
+      c = RemoteShell::PythonCommand.new($vm, code, user: @opts[:user])
+      if c.failure?
+        raise RuntimeError.new("The Dogtail script raised: #{c.exception}")
+      end
+      return c
     end
 
-    def run(lines = nil)
-      @opts[:user] ||= LIVE_USER
-      lines ||= [build_line]
-      lines = [lines] if lines.class != Array
-      script = build_script(lines)
-      script_path = $vm.execute_successfully('mktemp', @opts).stdout.chomp
-      $vm.file_overwrite(script_path, script)
-      args = ["/usr/bin/python '#{script_path}'", @opts]
-      if @opts[:allow_failure]
-        return $vm.execute(*args)
-      else
-        begin
-          return $vm.execute_successfully(*args)
-        rescue Exception => e
-          debug_log("Failing Dogtail script (#{script_path}):")
-          script.split("\n").each { |line| debug_log(" "*4 + line) }
-          raise e
-        end
-      end
+    def exist?
+      run("dogtail.config.searchCutoffCount = 0")
+      run(@find_code)
+      return true
+    rescue
+      return false
+    ensure
+      run("dogtail.config.searchCutoffCount = 20")
     end
 
     def self.value_to_s(v)
@@ -132,28 +124,6 @@ module Dogtail
       ).join(', ')
     end
 
-    def wait(timeout = nil)
-      if timeout
-        try_for(timeout) { run }
-      else
-        run
-      end
-    end
-
-    def exist?
-      @opts[:allow_failure] = true
-      # We do not want any retries since this method should return the
-      # result for the immediate situation, not for the situation up
-      # to 20 retries in the future.
-      optimization = "config.searchCutoffCount = 1"
-      @init_lines << optimization unless @init_lines.include?(optimization)
-      run.success?
-    end
-
-    def wait_vanish(timeout)
-      try_for(timeout) { not(exist?) }
-    end
-
     # Equivalent to the Tree API's Node.findChildren(), with the
     # arguments constructing a GenericPredicate to use as parameter.
     def children(*args)
@@ -173,50 +143,24 @@ module Dogtail
       if findChildren_opts_hash.size > 0
         findChildren_opts = ", " + self.class.args_to_s([findChildren_opts_hash])
       end
-      # A fundamental assumption of ScriptProxy is that we will only
-      # act on *one* object at a time. If we were to allow more, we'd
-      # have to port looping, conditionals and much more into our
-      # script generation, which is insane.
-      # However, since references are lost between script runs (=
-      # Application.run()) we need to be a bit tricky here. We use the
-      # internal a11y AT-SPI "path" to uniquely identify a Dogtail
-      # node, so we can give handles to each of them that can be used
-      # later to re-find them.
       predicate_opts = self.class.args_to_s(args)
-      find_paths_script_lines = [
-        "from dogtail import predicate",
-        "for n in #{build_line}.findChildren(predicate.GenericPredicate(#{predicate_opts})#{findChildren_opts}):",
-        "    print(n.path)",
+      nodes_var = "nodes#{@@node_counter += 1}"
+      find_script_lines = [
+        "#{nodes_var} = #{@var}.findChildren(dogtail.predicate.GenericPredicate(#{predicate_opts})#{findChildren_opts})",
+        "print(len(#{nodes_var}))",
       ]
-      a11y_at_spi_paths = run(find_paths_script_lines).stdout.chomp.split("\n")
-                          .grep(Regexp.new('^/org/a11y/atspi/accessible/'))
-                          .map { |path| path.chomp }
-      a11y_at_spi_paths.map do |path|
-        more_init_lines = [
-          "from dogtail import predicate",
-          "node = None",
-          "for n in #{build_line}.findChildren(predicate.GenericPredicate()):",
-          "    if str(n.path) == '#{path}':",
-          "        node = n",
-          "        break",
-          "assert(node)",
-        ]
-        Node.new(
-          @app_name,
-          @opts.merge(
-            init_lines: @init_lines + more_init_lines,
-            components: ['node']
-          )
-        )
+      size = run(find_script_lines).stdout.chomp.to_i
+      return size.times.map do |i|
+        Node.new("#{nodes_var}[#{i}]", @opts)
       end
     end
 
     def get_field(key)
-      run("print(#{build_line}.#{key})").stdout.chomp
+      run("print(#{@var}.#{key})").stdout.chomp
     end
 
     def set_field(key, value)
-      run("#{build_line}.#{key} = #{self.class.value_to_s(value)}")
+      run("#{@var}.#{key} = #{self.class.value_to_s(value)}")
     end
 
     def text
@@ -231,33 +175,17 @@ module Dogtail
       get_field('name')
     end
 
-    def proxy_call(method, args)
-      args_str = self.class.args_to_s(args)
-      method_call = "#{method.to_s}(#{args_str})"
-      Node.new(
-        @app_name,
-        @opts.merge(
-          init_lines: @init_lines,
-          components: @components + [method_call]
-        )
-      )
-    end
-
     TREE_API_APP_SEARCHES.each do |method|
       define_method(method) do |*args|
-        proxy_call(method, args)
+        args_str = self.class.args_to_s(args)
+        method_call = "#{method.to_s}(#{args_str})"
+        Node.new("#{@var}.#{method_call}", @opts)
       end
     end
 
     TREE_API_NODE_SEARCH_FIELDS.each do |field|
       define_method(field) do
-        Node.new(
-          @app_name,
-          @opts.merge(
-            init_lines: @init_lines,
-            components: @components + [field]
-          )
-        )
+        Node.new("#{@var}.#{field}", @opts)
       end
     end
 
@@ -265,15 +193,28 @@ module Dogtail
 
   class Node < Application
 
+    def initialize(expr, opts = {})
+      @expr = expr
+      @opts = opts
+      @opts[:user] ||= LIVE_USER
+      @find_code = expr
+      @var = "node#{@@node_counter += 1}"
+      run("#{@var} = #{@find_code}")
+    end
+
     TREE_API_NODE_SEARCHES.each do |method|
       define_method(method) do |*args|
-        proxy_call(method, args)
+        args_str = self.class.args_to_s(args)
+        method_call = "#{method.to_s}(#{args_str})"
+        Node.new("#{@var}.#{method_call}", @opts)
       end
     end
 
     TREE_API_NODE_ACTIONS.each do |method|
       define_method(method) do |*args|
-        proxy_call(method, args).run
+        args_str = self.class.args_to_s(args)
+        method_call = "#{method.to_s}(#{args_str})"
+        run("#{@var}.#{method_call}")
       end
     end
 
diff --git a/features/support/helpers/remote_shell.rb b/features/support/helpers/remote_shell.rb
index b2465a3..b890578 100644
--- a/features/support/helpers/remote_shell.rb
+++ b/features/support/helpers/remote_shell.rb
@@ -56,7 +56,7 @@ module RemoteShell
   module_function :communicate
   private :communicate
 
-  class Command
+  class ShellCommand
     # If `:spawn` is false the server will block until it has finished
     # executing `cmd`. If it's true the server won't block, and the
     # response will always be [0, "", ""] (only used as an
@@ -69,7 +69,7 @@ module RemoteShell
       opts[:spawn] = false unless opts.has_key?(:spawn)
       type = opts[:spawn] ? "spawn" : "call"
       debug_log("#{type}ing as #{opts[:user]}: #{cmd}")
-      ret = RemoteShell.communicate(vm, type, opts[:user], cmd, **opts)
+      ret = RemoteShell.communicate(vm, 'sh_' + type, opts[:user], cmd, **opts)
       debug_log("#{type} returned: #{ret}") if not(opts[:spawn])
       return ret
     end
@@ -98,12 +98,51 @@ module RemoteShell
     end
   end
 
+  class PythonCommand
+    def self.execute(vm, code, **opts)
+      opts[:user] ||= "root"
+      show_code = code.chomp
+      if show_code["\n"]
+        show_code = "\n" + show_code.lines.map { |l| " "*4 + l.chomp } .join("\n")
+      end
+      debug_log("executing Python as #{opts[:user]}: #{show_code}")
+      ret = RemoteShell.communicate(
+        vm, 'python_execute', opts[:user], code, **opts
+      )
+      debug_log("execution complete")
+      return ret
+    end
+
+    attr_reader :code, :exception, :stdout, :stderr
+
+    def initialize(vm, code, **opts)
+      @code = code
+      @exception, @stdout, @stderr = self.class.execute(vm, code, **opts)
+    end
+
+    def success?
+      return @exception == nil
+    end
+
+    def failure?
+      return not(success?)
+    end
+
+    def to_s
+      "Exception: #{@exception}\n" +
+        "STDOUT:\n" +
+        @stdout +
+        "STDERR:\n" +
+        @stderr
+    end
+  end
+
   # An IO-like object that is more or less equivalent to a File object
   # opened in rw mode.
   class File
     def self.open(vm, mode, path, *args, **opts)
       debug_log("opening file #{path} in '#{mode}' mode")
-      ret = RemoteShell.communicate(vm, mode, path, *args, **opts)
+      ret = RemoteShell.communicate(vm, 'file_' + mode, path, *args, **opts)
       if ret.size != 1
         raise ServerFailure.new("expected 1 value but got #{ret.size}")
       end
diff --git a/features/support/helpers/vm_helper.rb b/features/support/helpers/vm_helper.rb
index 3029acf..bbfdfa6 100644
--- a/features/support/helpers/vm_helper.rb
+++ b/features/support/helpers/vm_helper.rb
@@ -463,7 +463,7 @@ EOF
       cmds << cmd
       cmd = cmds.join(" && ")
     end
-    return RemoteShell::Command.new(self, cmd, options)
+    return RemoteShell::ShellCommand.new(self, cmd, options)
   end
 
   def execute_successfully(*args)