Browse Source

FileManager/ReaderUI: Clarify the current instance accessor (#7658)

* FileManager/ReaderUI: Clarify the current instance accessor

Make it clearer that we actually store it in a *module/class* member, not an *instance* member.

Also, warn if there's a close/open mismatch.
reviewable/pr7668/r1
NiLuJe 5 months ago
committed by GitHub
parent
commit
de6f2e84a3
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 65 additions and 40 deletions
  1. +33
    -19
      frontend/apps/filemanager/filemanager.lua
  2. +25
    -8
      frontend/apps/reader/readerui.lua
  3. +6
    -4
      frontend/ui/widget/filechooser.lua
  4. +1
    -8
      plugins/opds.koplugin/opdscatalog.lua
  5. +0
    -1
      reader.lua

+ 33
- 19
frontend/apps/filemanager/filemanager.lua View File

@ -29,7 +29,6 @@ local PluginLoader = require("pluginloader")
local ReadCollection = require("readcollection")
local ReaderDeviceStatus = require("apps/reader/modules/readerdevicestatus")
local ReaderDictionary = require("apps/reader/modules/readerdictionary")
local ReaderUI = require("apps/reader/readerui")
local ReaderWikipedia = require("apps/reader/modules/readerwikipedia")
local Screenshoter = require("ui/widget/screenshoter")
local Size = require("ui/size")
@ -50,7 +49,6 @@ local T = BaseUtil.template
local FileManager = InputContainer:extend{
title = _("KOReader"),
root_path = lfs.currentdir(),
onExit = function() end,
mv_bin = Device:isAndroid() and "/system/bin/mv" or "/bin/mv",
cp_bin = Device:isAndroid() and "/system/bin/cp" or "/bin/cp",
@ -60,10 +58,10 @@ local FileManager = InputContainer:extend{
function FileManager:onSetRotationMode(rotation)
if rotation ~= nil and rotation ~= Screen:getRotationMode() then
Screen:setRotationMode(rotation)
if self.instance then
self:reinit(self.instance.path, self.instance.focused_file)
UIManager:setDirty(self.instance.banner, function()
return "ui", self.instance.banner.dimen
if FileManager.instance then
self:reinit(self.path, self.focused_file)
UIManager:setDirty(self.banner, function()
return "ui", self.banner.dimen
end)
end
end
@ -215,6 +213,7 @@ function FileManager:setupLayout()
end
function file_chooser:onFileSelect(file) -- luacheck: ignore
local ReaderUI = require("apps/reader/readerui")
ReaderUI:showReader(file)
return true
end
@ -396,7 +395,7 @@ function FileManager:setupLayout()
end,
})
end
self:showSetProviderButtons(file, FileManager.instance, ReaderUI, one_time_providers)
self:showSetProviderButtons(file, FileManager.instance, one_time_providers)
end,
},
{
@ -737,8 +736,8 @@ function FileManager:reinit(path, focused_file)
end
function FileManager:getCurrentDir()
if self.instance then
return self.instance.file_chooser.path
if FileManager.instance then
return FileManager.instance.file_chooser.path
end
end
@ -768,12 +767,18 @@ function FileManager:onClose()
self:handleEvent(Event:new("SaveSettings"))
G_reader_settings:flush()
UIManager:close(self)
if self.onExit then
self:onExit()
end
return true
end
function FileManager:onCloseWidget()
if FileManager.instance == self then
logger.dbg("Tearing down FileManager", tostring(self))
else
logger.warn("FileManager instance mismatch! Closed", tostring(self), "while the active one is supposed to be", tostring(FileManager.instance))
end
FileManager.instance = nil
end
function FileManager:onShowingReader()
-- Allows us to optimize out a few useless refreshes in various CloseWidgets handlers...
self.tearing_down = true
@ -834,6 +839,7 @@ function FileManager:openRandomFile(dir)
text = T(_("Do you want to open %1?"), BD.filename(BaseUtil.basename(random_file))),
choice1_text = _("Open"),
choice1_callback = function()
local ReaderUI = require("apps/reader/readerui")
ReaderUI:showReader(random_file)
end,
-- @translators Another file. This is a button on the open random file dialog. It presents a file with the choices Open/Another.
@ -1145,6 +1151,13 @@ function FileManager:getStartWithMenuTable()
end
function FileManager:showFiles(path, focused_file)
-- Warn about and close any pre-existing FM instances first...
if FileManager.instance then
logger.warn("FileManager instance mismatch! Tried to spin up a new instance, while we still have an existing one:", tostring(FileManager.instance))
-- Close the old one first!
FileManager.instance:onClose()
end
path = path or G_reader_settings:readSetting("lastdir") or filemanagerutil.getDefaultDir()
G_reader_settings:saveSetting("lastdir", path)
self:setRotationMode()
@ -1153,16 +1166,17 @@ function FileManager:showFiles(path, focused_file)
covers_fullscreen = true, -- hint for UIManager:_repaint()
root_path = path,
focused_file = focused_file,
onExit = function()
self.instance = nil
end
}
UIManager:show(file_manager)
-- NOTE: This is a bit clunky. This ought to be private and accessed via a getCurrentInstance method, àla ReaderUI.
-- But, it points to the *current* FM instance, and is nil'ed on exit.
-- As such, code outside of FileManager can just check/use FileManager.instance (which they do. extensively).
self.instance = file_manager
-- NOTE: ReaderUI has a _getRunningInstance method for this, because it used to store the instance reference in a private module variable.
if FileManager.instance == nil then
logger.dbg("Spinning up new FileManager instance", tostring(file_manager))
else
-- Should never happen, given what we did above...
logger.warn("FileManager instance mismatch! Opened", tostring(file_manager), "while we still have an existing instance:", tostring(FileManager.instance))
end
FileManager.instance = file_manager
end
--- A shortcut to execute mv.

+ 25
- 8
frontend/apps/reader/readerui.lua View File

@ -567,12 +567,12 @@ function ReaderUI:showReaderCoroutine(file, provider)
end)
end
local _running_instance = nil
function ReaderUI:doShowReader(file, provider)
logger.info("opening file", file)
-- Keep only one instance running
if _running_instance then
_running_instance:onClose()
-- Only keep a single instance running
if ReaderUI.instance then
logger.warn("ReaderUI instance mismatch! Tried to spin up a new instance, while we still have an existing one:", tostring(ReaderUI.instance))
ReaderUI.instance:onClose()
end
local document = DocumentRegistry:openDocument(file, provider)
if not document then
@ -621,11 +621,21 @@ function ReaderUI:doShowReader(file, provider)
end
UIManager:show(reader, "full")
_running_instance = reader
if ReaderUI.instance == nil then
logger.dbg("Spinning up new ReaderUI instance", tostring(reader))
else
-- Should never happen, given what we did above...
logger.warn("ReaderUI instance mismatch! Opened", tostring(reader), "while we still have an existing instance:", tostring(ReaderUI.instance))
end
ReaderUI.instance = reader
end
-- NOTE: The instance reference used to be stored in a private module variable, hence the getter method.
-- We've since aligned behavior with FileManager, which uses a class member instead,
-- but kept the function to avoid changing existing code.
function ReaderUI:_getRunningInstance()
return _running_instance
return ReaderUI.instance
end
function ReaderUI:unlockDocumentWithPassword(document, try_again)
@ -741,9 +751,16 @@ function ReaderUI:onClose(full_refresh)
self:notifyCloseDocument()
end
UIManager:close(self.dialog, full_refresh and "full")
if _running_instance == self then
_running_instance = nil
end
function ReaderUI:onCloseWidget()
if ReaderUI.instance == self then
logger.dbg("Tearing down ReaderUI", tostring(self))
else
logger.warn("ReaderUI instance mismatch! Closed", tostring(self), "while the active one is supposed to be", tostring(ReaderUI.instance))
end
ReaderUI.instance = nil
self._coroutine = nil
end
function ReaderUI:dealWithLoadDocumentFailure()

+ 6
- 4
frontend/ui/widget/filechooser.lua View File

@ -470,7 +470,9 @@ function FileChooser:getNextFile(curr_file)
return next_file
end
function FileChooser:showSetProviderButtons(file, filemanager_instance, reader_ui, one_time_providers)
function FileChooser:showSetProviderButtons(file, filemanager_instance, one_time_providers)
local ReaderUI = require("apps/reader/readerui")
local __, filename_pure = util.splitFilePathName(file)
local filename_suffix = util.getFileNameSuffix(file)
@ -540,7 +542,7 @@ function FileChooser:showSetProviderButtons(file, filemanager_instance, reader_u
ok_callback = function()
DocumentRegistry:setProvider(file, provider, false)
reader_ui:showReader(file, provider)
ReaderUI:showReader(file, provider)
UIManager:close(self.set_provider_dialog)
end,
})
@ -553,13 +555,13 @@ function FileChooser:showSetProviderButtons(file, filemanager_instance, reader_u
ok_callback = function()
DocumentRegistry:setProvider(file, provider, true)
reader_ui:showReader(file, provider)
ReaderUI:showReader(file, provider)
UIManager:close(self.set_provider_dialog)
end,
})
else
-- just once
reader_ui:showReader(file, provider)
ReaderUI:showReader(file, provider)
UIManager:close(self.set_provider_dialog)
end
end,

+ 1
- 8
plugins/opds.koplugin/opdscatalog.lua View File

@ -4,7 +4,6 @@ local ConfirmBox = require("ui/widget/confirmbox")
local FrameContainer = require("ui/widget/container/framecontainer")
local InputContainer = require("ui/widget/container/inputcontainer")
local OPDSBrowser = require("opdsbrowser")
local ReaderUI = require("apps/reader/readerui")
local UIManager = require("ui/uimanager")
local logger = require("logger")
local _ = require("gettext")
@ -13,7 +12,6 @@ local T = require("ffi/util").template
local OPDSCatalog = InputContainer:extend{
title = _("OPDS Catalog"),
onExit = function() end,
}
function OPDSCatalog:init()
@ -36,6 +34,7 @@ function OPDSCatalog:init()
self:onClose()
local ReaderUI = require("apps/reader/readerui")
ReaderUI:showReader(downloaded_file)
end
})
@ -67,18 +66,12 @@ function OPDSCatalog:showCatalog()
UIManager:show(OPDSCatalog:new{
dimen = Screen:getSize(),
covers_fullscreen = true, -- hint for UIManager:_repaint()
onExit = function()
--UIManager:quit()
end
})
end
function OPDSCatalog:onClose()
logger.dbg("close OPDS catalog")
UIManager:close(self)
if self.onExit then
self:onExit()
end
return true
end

+ 0
- 1
reader.lua View File

@ -342,7 +342,6 @@ local function exitReader()
-- Save current rotation (or the original rotation if ScreenSaver temporarily modified it) to remember it for next startup
G_reader_settings:saveSetting("closed_rotation_mode", Device.orig_rotation_mode or Device.screen:getRotationMode())
G_reader_settings:close()
-- Close lipc handles

Loading…
Cancel
Save