Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add window refresh lock to block window from updating while handler is running. #2955

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/2955.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Windows now have a refresh lock to stop refreshing window layout happening after every add()
12 changes: 10 additions & 2 deletions core/src/toga/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ async def handler_with_cleanup(
**kwargs: object,
) -> object | None:
try:
result = await handler(interface, *args, **kwargs)
if hasattr(interface, "window") and interface.window is not None:
with interface.window.refresh_lock():
result = await handler(interface, *args, **kwargs)
else:
result = await handler(interface, *args, **kwargs)
except Exception as e:
print("Error in async handler:", e, file=sys.stderr)
traceback.print_exc()
Expand Down Expand Up @@ -170,7 +174,11 @@ def _handler(*args: object, **kwargs: object) -> object:
)
else:
try:
result = handler(interface, *args, **kwargs)
if hasattr(interface, "window") and interface.window is not None:
with interface.window.refresh_lock():
result = handler(interface, *args, **kwargs)
else:
result = handler(interface, *args, **kwargs)
except Exception as e:
print("Error in handler:", e, file=sys.stderr)
traceback.print_exc()
Expand Down
4 changes: 4 additions & 0 deletions core/src/toga/widgets/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,10 @@ def enabled(self, value: bool) -> None:
self._impl.set_enabled(bool(value))

def refresh(self) -> None:
if self.window is not None and self.window.locked:
self.window.dirty(self)
return

self._impl.refresh()

# Refresh the layout
Expand Down
37 changes: 37 additions & 0 deletions core/src/toga/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from toga.types import PositionT, SizeT
from toga.widgets.base import Widget

from contextlib import contextmanager

_window_count = -1

Expand Down Expand Up @@ -190,6 +191,8 @@ def __init__(
self._content: Widget | None = None
self._is_full_screen = False
self._closed = False
self._locked = False
self._dirty = set()

self._resizable = resizable
self._closable = closable
Expand Down Expand Up @@ -353,6 +356,8 @@ def content(self) -> Widget | None:
def content(self, widget: Widget) -> None:
# Set window of old content to None
if self._content:
if self._content in self._dirty:
self._dirty.remove(self._content)
self._content.window = None

# Assign the content widget to the same app as the window.
Expand Down Expand Up @@ -393,6 +398,38 @@ def size(self, size: SizeT) -> None:
if self.content:
self.content.refresh()

def dirty(self, widget: Widget) -> None:
"""Mark widget as being 'dirty', so that it gets refreshed
when the window lock is lifted
"""
self._dirty.add(widget)

@property
def locked(self) -> bool:
"""Is the window locked from refreshes?"""
return self._locked

@locked.setter
def locked(self, locked: bool) -> None:
"""Lock or unlock window refresh.
When window is unlocked, refresh the content
"""
self._locked = locked
if not locked:
for child in self._dirty:
child.refresh()

self._dirty.clear()

@contextmanager
def refresh_lock(self, *args, **kwargs):
"""Obtain a refresh lock on the window. Unlocks
automatically when context manager leaves scope.
"""
self.locked = True
yield
self.locked = False

######################################################################
# Window position
######################################################################
Expand Down
120 changes: 99 additions & 21 deletions core/tests/test_handlers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import asyncio
from unittest.mock import Mock
from unittest.mock import MagicMock, Mock

import pytest

Expand Down Expand Up @@ -61,8 +61,14 @@ def test_noop_handler_with_cleanup_error(capsys):


def test_function_handler():
"""A function can be used as a handler."""
obj = Mock()
"""A function can be used as a handler"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
handler_call = {}

def handler(*args, **kwargs):
Expand All @@ -86,8 +92,14 @@ def handler(*args, **kwargs):


def test_function_handler_error(capsys):
"""A function handler can raise an error."""
obj = Mock()
"""A function handler can raise an error"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
handler_call = {}

def handler(*args, **kwargs):
Expand Down Expand Up @@ -116,8 +128,14 @@ def handler(*args, **kwargs):


def test_function_handler_with_cleanup():
"""A function handler can have a cleanup method."""
obj = Mock()
"""A function handler can have a cleanup method"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
cleanup = Mock()
handler_call = {}

Expand Down Expand Up @@ -146,7 +164,13 @@ def handler(*args, **kwargs):

def test_function_handler_with_cleanup_error(capsys):
"""A function handler can have a cleanup method that raises an error."""
obj = Mock()
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
cleanup = Mock(side_effect=Exception("Problem in cleanup"))
handler_call = {}

Expand Down Expand Up @@ -180,8 +204,14 @@ def handler(*args, **kwargs):


def test_generator_handler(event_loop):
"""A generator can be used as a handler."""
obj = Mock()
"""A generator can be used as a handler"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
handler_call = {}

def handler(*args, **kwargs):
Expand Down Expand Up @@ -213,8 +243,14 @@ def handler(*args, **kwargs):


def test_generator_handler_error(event_loop, capsys):
"""A generator can raise an error."""
obj = Mock()
"""A generator can raise an error"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
handler_call = {}

def handler(*args, **kwargs):
Expand Down Expand Up @@ -248,8 +284,14 @@ def handler(*args, **kwargs):


def test_generator_handler_with_cleanup(event_loop):
"""A generator can have cleanup."""
obj = Mock()
"""A generator can have cleanup"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
cleanup = Mock()
handler_call = {}

Expand Down Expand Up @@ -285,8 +327,14 @@ def handler(*args, **kwargs):


def test_generator_handler_with_cleanup_error(event_loop, capsys):
"""A generator can raise an error during cleanup."""
obj = Mock()
"""A generator can raise an error during cleanup"""
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
cleanup = Mock(side_effect=Exception("Problem in cleanup"))
handler_call = {}

Expand Down Expand Up @@ -329,7 +377,13 @@ def handler(*args, **kwargs):

def test_coroutine_handler(event_loop):
"""A coroutine can be used as a handler."""
obj = Mock()
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
handler_call = {}

async def handler(*args, **kwargs):
Expand Down Expand Up @@ -359,7 +413,13 @@ async def handler(*args, **kwargs):

def test_coroutine_handler_error(event_loop, capsys):
"""A coroutine can raise an error."""
obj = Mock()
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
handler_call = {}

async def handler(*args, **kwargs):
Expand Down Expand Up @@ -394,7 +454,13 @@ async def handler(*args, **kwargs):

def test_coroutine_handler_with_cleanup(event_loop):
"""A coroutine can have cleanup."""
obj = Mock()
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
cleanup = Mock()
handler_call = {}

Expand Down Expand Up @@ -428,7 +494,13 @@ async def handler(*args, **kwargs):

def test_coroutine_handler_with_cleanup_error(event_loop, capsys):
"""A coroutine can raise an error during cleanup."""
obj = Mock()
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
cleanup = Mock(side_effect=Exception("Problem in cleanup"))
handler_call = {}

Expand Down Expand Up @@ -468,7 +540,13 @@ async def handler(*args, **kwargs):

def test_native_handler():
"""A native function can be used as a handler."""
obj = Mock()
obj = MagicMock()
obj.return_value.window.return_value.refresh_lock.return_value.__enter__.return_value = (
None
)
obj.return_value.window.return_value.refresh_lock.return_value.__exit__.return_value = (
None
)
native_method = Mock()

handler = NativeHandler(native_method)
Expand Down
Loading