[no squash] Get rid of the last remaining sync. HTTP requests on the main thread

builtin/mainmenu/content/contentdb.lua

@@ -181,14 +181,24 @@ function contentdb.get_package_by_id(id)
 end
 
 
-local function get_raw_dependencies(package)
- if package.type ~= "mod" then
- return {}
- end
- if package.raw_deps then
- return package.raw_deps
+local function make_callback_coroutine(fn, callback)
+ local co = coroutine.create(fn)
+
+ local function resumer(...)
+ local ok, result = coroutine.resume(co, ...)
+
+ if not ok then
+ error(result)
+ elseif coroutine.status(co) == "dead" then
+ callback(result)
+ end
  end
 
+ return resumer
+end
+
+
+local function get_raw_dependencies_async(package)
  local url_fmt = "/api/packages/%s/dependencies/?only_hard=1&protocol_version=%s&engine_version=%s"
  local version = core.get_version()
  local base_url = core.settings:get("contentdb_url")
@@ -197,11 +207,25 @@ local function get_raw_dependencies(package)
  local http = core.get_http_api()
  local response = http.fetch_sync({ url = url })
  if not response.succeeded then
- core.log("error", "Unable to fetch dependencies for " .. package.url_part)
- return
+ return nil
+ end
+ return core.parse_json(response.data) or {}
+end
+
+
+local function get_raw_dependencies_co(package, resumer)
+ if package.type ~= "mod" then
+ return {}
+ end
+ if package.raw_deps then
+ return package.raw_deps
  end
 
- local data = core.parse_json(response.data) or {}
+ core.handle_async(get_raw_dependencies_async, package, resumer)
+ local data = coroutine.yield()
+ if not data then
+ return nil
+ end
 
  for id, raw_deps in pairs(data) do
  local package2 = contentdb.package_by_id[id:lower()]
@@ -222,8 +246,8 @@ local function get_raw_dependencies(package)
 end
 
 
-function contentdb.has_hard_deps(package)
- local raw_deps = get_raw_dependencies(package)
+local function has_hard_deps_co(package, resumer)
+ local raw_deps = get_raw_dependencies_co(package, resumer)
  if not raw_deps then
  return nil
  end
@@ -238,8 +262,14 @@ function contentdb.has_hard_deps(package)
 end
 
 
+function contentdb.has_hard_deps(package, callback)
+ local resumer = make_callback_coroutine(has_hard_deps_co, callback)
+ resumer(package, resumer)
+end
+
+
 -- Recursively resolve dependencies, given the installed mods
-local function resolve_dependencies_2(raw_deps, installed_mods, out)
+local function resolve_dependencies_2_co(raw_deps, installed_mods, out, resumer)
  local function resolve_dep(dep)
  -- Check whether it's already installed
  if installed_mods[dep.name] then
@@ -289,9 +319,9 @@ local function resolve_dependencies_2(raw_deps, installed_mods, out)
  local result = resolve_dep(dep)
  out[dep.name] = result
  if result and result.package and not result.installed then
- local raw_deps2 = get_raw_dependencies(result.package)
+ local raw_deps2 = get_raw_dependencies_co(result.package, resumer)
  if raw_deps2 then
- resolve_dependencies_2(raw_deps2, installed_mods, out)
+ resolve_dependencies_2_co(raw_deps2, installed_mods, out, resumer)
  end
  end
  end
@@ -301,11 +331,10 @@ local function resolve_dependencies_2(raw_deps, installed_mods, out)
 end
 
 
--- Resolve dependencies for a package, calls the recursive version.
-function contentdb.resolve_dependencies(package, game)
+local function resolve_dependencies_co(package, game, resumer)
  assert(game)
 
- local raw_deps = get_raw_dependencies(package)
+ local raw_deps = get_raw_dependencies_co(package, resumer)
  local installed_mods = {}
 
  local mods = {}
@@ -319,7 +348,7 @@ function contentdb.resolve_dependencies(package, game)
  end
 
  local out = {}
- if not resolve_dependencies_2(raw_deps, installed_mods, out) then
+ if not resolve_dependencies_2_co(raw_deps, installed_mods, out, resumer) then
  return nil
  end
 
@@ -336,6 +365,13 @@ function contentdb.resolve_dependencies(package, game)
 end
 
 
+-- Resolve dependencies for a package, calls the recursive version.
+function contentdb.resolve_dependencies(package, game, callback)
+ local resumer = make_callback_coroutine(resolve_dependencies_co, callback)
+ resumer(package, game, resumer)
+end
+
+
 local function fetch_pkgs(params)
  local version = core.get_version()
  local base_url = core.settings:get("contentdb_url")

builtin/mainmenu/content/dlg_contentdb.lua

@@ -63,21 +63,12 @@ local function install_or_update_package(this, package)
  end
 
  local function on_confirm()
- local has_hard_deps = contentdb.has_hard_deps(package)
- if has_hard_deps then
- local dlg = create_install_dialog(package)
- dlg:set_parent(this)
- this:hide()
- dlg:show()
- elseif has_hard_deps == nil then
- local dlg = messagebox("error_checking_deps",
- fgettext("Error getting dependencies for package"))
- dlg:set_parent(this)
- this:hide()
- dlg:show()
- else
- contentdb.queue_download(package, package.path and contentdb.REASON_UPDATE or contentdb.REASON_NEW)
- end
+ local dlg = create_install_dialog(package)
+ dlg:set_parent(this)
+ this:hide()
+ dlg:show()
+
+ dlg:load_deps()
  end
 
  if package.type == "mod" and #pkgmgr.games == 0 then

builtin/mainmenu/content/dlg_install.lua

@@ -15,7 +15,31 @@
 --with this program; if not, write to the Free Software Foundation, Inc.,
 --51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
+local function is_still_visible(dlg)
+ local this = ui.find_by_name("install_dialog")
+ return this == dlg and not dlg.hidden
+end
+
+
+local function get_loading_formspec()
+ local ENABLE_TOUCH = core.settings:get_bool("enable_touch")
+ local w = ENABLE_TOUCH and 14 or 7
+
+ local formspec = {
+ "formspec_version[3]",
+ "size[", w, ",9.05]",
+ ENABLE_TOUCH and "padding[0.01,0.01]" or "position[0.5,0.55]",
+ "label[3,4.525;", fgettext("Loading..."), "]",
+ }
+ return table.concat(formspec)
+end
+
+
 local function get_formspec(data)
+ if not data.has_hard_deps_ready then
+ return get_loading_formspec()
+ end
+
  local selected_game, selected_game_idx = pkgmgr.find_by_gameid(core.settings:get("menu_last_game"))
  if not selected_game_idx then
  selected_game_idx = 1
@@ -27,15 +51,35 @@ local function get_formspec(data)
  game_list[i] = core.formspec_escape(game.title)
  end
 
+ if not data.deps_ready[selected_game_idx] and
+ not data.deps_loading[selected_game_idx] then
+ data.deps_loading[selected_game_idx] = true
+
+ contentdb.resolve_dependencies(data.package, selected_game, function(deps)
+ if not is_still_visible(data.dlg) then
+ return
+ end
+ data.deps_ready[selected_game_idx] = deps
+ ui.update()
+ end)
+ end
+
+ -- The value of `data.deps_ready[selected_game_idx]` may have changed
+ -- since the last if statement since `contentdb.resolve_dependencies`
+ -- calls the callback immediately if the dependencies are already cached.
+ if not data.deps_ready[selected_game_idx] then
+ return get_loading_formspec()
+ end
+
  local package = data.package
  local will_install_deps = data.will_install_deps
 
  local deps_to_install = 0
  local deps_not_found = 0
 
- data.dependencies = contentdb.resolve_dependencies(package, selected_game)
+ data.deps_chosen = data.deps_ready[selected_game_idx]
  local formatted_deps = {}
- for _, dep in pairs(data.dependencies) do
+ for _, dep in pairs(data.deps_chosen) do
  formatted_deps[#formatted_deps + 1] = "#fff"
  formatted_deps[#formatted_deps + 1] = core.formspec_escape(dep.name)
  if dep.installed then
@@ -128,7 +172,7 @@ local function handle_submit(this, fields)
  contentdb.queue_download(data.package, contentdb.REASON_NEW)
 
  if data.will_install_deps then
- for _, dep in pairs(data.dependencies) do
+ for _, dep in pairs(data.deps_chosen) do
  if not dep.is_optional and not dep.installed and dep.package then
  contentdb.queue_download(dep.package, contentdb.REASON_DEPENDENCY)
  end
@@ -153,10 +197,50 @@ local function handle_submit(this, fields)
 end
 
 
+local function load_deps(dlg)
+ local package = dlg.data.package
+
+ contentdb.has_hard_deps(package, function(result)
+ if not is_still_visible(dlg) then
+ return
+ end
+
+ if result == nil then
+ local parent = dlg.parent
+ dlg:delete()
+ local dlg2 = messagebox("error_checking_deps",
+ fgettext("Error getting dependencies for package $1", package.url_part))
+ dlg2:set_parent(parent)
+ parent:hide()
+ dlg2:show()
+ elseif result == false then
+ contentdb.queue_download(package, package.path and contentdb.REASON_UPDATE or contentdb.REASON_NEW)
+ dlg:delete()
+ else
+ assert(result == true)
+ dlg.data.has_hard_deps_ready = true
+ end
+ ui.update()
+ end)
+end
+
+
 function create_install_dialog(package)
  local dlg = dialog_create("install_dialog", get_formspec, handle_submit, nil)
- dlg.data.dependencies = nil
+ dlg.data.deps_chosen = nil
  dlg.data.package = package
  dlg.data.will_install_deps = true
+
+ dlg.data.has_hard_deps_ready = false
+ dlg.data.deps_ready = {}
+ dlg.data.deps_loading = {}
+
+ dlg.load_deps = load_deps
+
+ -- `get_formspec` needs to access `dlg` to check whether it's still open.
+ -- It doesn't suffice to check that any "install_dialog" instance is open
+ -- via `ui.find_by_name`, it's necessary to check for this exact instance.
+ dlg.data.dlg = dlg
+
  return dlg
 end

src/httpfetch.cpp

@@ -749,19 +749,6 @@ static void httpfetch_request_clear(u64 caller)
  }
 }
 
-static void httpfetch_sync(const HTTPFetchRequest &fetch_request,
- HTTPFetchResult &fetch_result)
-{
- // Create ongoing fetch data and make a cURL handle
- // Set cURL options based on HTTPFetchRequest
- CurlHandlePool pool;
- HTTPFetchOngoing ongoing(fetch_request, &pool);
- // Do the fetch (curl_easy_perform)
- CURLcode res = ongoing.start(nullptr);
- // Update fetch result
- fetch_result = *ongoing.complete(res);
-}
-
 bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
  HTTPFetchResult &fetch_result, long interval)
 {
@@ -779,7 +766,8 @@ bool httpfetch_sync_interruptible(const HTTPFetchRequest &fetch_request,
  } while (!httpfetch_async_get(req.caller, fetch_result));
  httpfetch_caller_free(req.caller);
  } else {
- httpfetch_sync(fetch_request, fetch_result);
+ throw ModError(std::string("You have tried to execute a synchronous HTTP request on the main thread! "
+ "This offense shall be punished. (").append(fetch_request.url).append(")"));
  }
  return true;
 }