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

Make upstream window and app references weak to avoid reference cycles #2061

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d3fd16d
Update docstrings and documentation for Window and MainWindow.
freakboy3742 Jul 31, 2023
8c09ef1
Add changenote.
freakboy3742 Jul 31, 2023
d2eec81
Migrated core tests to pytest.
freakboy3742 Aug 1, 2023
7ee6369
Complete coverage of core dialogs.
freakboy3742 Aug 1, 2023
3e6bf86
Remove GTK window tests.
freakboy3742 Aug 1, 2023
cec300d
Correct a spelling issue, and document an API rename.
freakboy3742 Aug 1, 2023
a13683b
Exclude some clases from interface tests.
freakboy3742 Aug 1, 2023
32097ca
Cocoa GUI tests for Window (excluding dialogs and toolbars.
freakboy3742 Aug 1, 2023
212fb89
Add menu items and keyboard shortcuts for close/minimize on macOS.
freakboy3742 Aug 1, 2023
f3bffde
Add GUI test of full screen mode.
freakboy3742 Aug 2, 2023
86e91ae
Converge on US spelling for resizable, closable
freakboy3742 Aug 2, 2023
9f719f6
Cocoa dialogs 100% coverage.
freakboy3742 Aug 2, 2023
e3613f2
100% coverage for Window on iOS.
freakboy3742 Aug 2, 2023
7cd3fde
Minor coverage tweaks.
freakboy3742 Aug 2, 2023
c867585
GTK Window and Dialogs to 100%.
freakboy3742 Aug 3, 2023
066a384
Tweak visibility handling to ensure complete coverage in testbed cond…
freakboy3742 Aug 3, 2023
3bd0b05
Use blackbox as a testing WM.
freakboy3742 Aug 4, 2023
054658d
Rework file dialog setup handling to avoid CI failure.
freakboy3742 Aug 4, 2023
e18c5b1
More test updates for CI's benefit.
freakboy3742 Aug 4, 2023
59cfb50
Restructure menu creation for cocoa.
freakboy3742 Aug 5, 2023
4fffdbe
Rework default title handling so MainWindow defaults to the formal na…
freakboy3742 Aug 5, 2023
ce4130d
Add a test of closing window explicitly.
freakboy3742 Aug 11, 2023
a04ad63
Merge branch 'main' into audit-window
freakboy3742 Aug 16, 2023
cc2df8b
Insert a pause on app exit to make sure Briefcase gets all the app logs.
freakboy3742 Aug 16, 2023
f8a8773
Make app and window back references weak.
freakboy3742 Aug 4, 2023
b6fdd2d
Roll out weakref change across other backends.
freakboy3742 Aug 7, 2023
c9ed0f8
Add weakrefs to dummy, and a test of GC on the live backends.
freakboy3742 Aug 7, 2023
73714f6
Corrected some inheritance issues with iOS containers.
freakboy3742 Aug 10, 2023
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
11 changes: 8 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,15 @@ jobs:
- backend: linux
runs-on: ubuntu-22.04
# The package list should be the same as in tutorial-0.rst, and the BeeWare
# tutorial, plus flwm to provide a window manager
# tutorial, plus blackbox to provide a window manager. We need a window
# manager that is reasonably lightweight, honors full screen mode, and
# treats the window position as the top-left corner of the *window*, not the
# top-left corner of the window *content*. The default GNOME window managers of
# most distros meet these requirementt, but they're heavyweight; flwm doesn't
# work either. Blackbox is the lightest WM we've found that works.
pre-command: |
sudo apt update -y
sudo apt install -y flwm pkg-config python3-dev libgirepository1.0-dev libcairo2-dev gir1.2-webkit2-4.0
sudo apt install -y blackbox pkg-config python3-dev libgirepository1.0-dev libcairo2-dev gir1.2-webkit2-4.0

# Start Virtual X server
echo "Start X server..."
Expand All @@ -219,7 +224,7 @@ jobs:

# Start Window manager
echo "Start window manager..."
DISPLAY=:99 flwm &
DISPLAY=:99 blackbox &
sleep 1

briefcase-run-prefix: 'DISPLAY=:99'
Expand Down
1 change: 1 addition & 0 deletions 1215.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A memory leak associated with creation and deletion of windows has been resolved.
10 changes: 9 additions & 1 deletion android/src/toga_android/app.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import weakref

from rubicon.java import android_events

Expand Down Expand Up @@ -173,7 +174,6 @@ def native(self):
class App:
def __init__(self, interface):
self.interface = interface
self.interface._impl = self
self._listener = None

self.loop = android_events.AndroidEventLoop()
Expand All @@ -182,6 +182,14 @@ def __init__(self, interface):
def native(self):
return self._listener.native if self._listener else None

@property
def interface(self):
return self._interface()

@interface.setter
def interface(self, value):
self._interface = weakref.ref(value)

def create(self):
# The `_listener` listens for activity event callbacks. For simplicity,
# the app's `.native` is the listener's native Java class.
Expand Down
11 changes: 10 additions & 1 deletion android/src/toga_android/window.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import weakref

from decimal import ROUND_UP

from .container import Container
Expand All @@ -23,9 +25,16 @@ class Window(Container):
def __init__(self, interface, title, position, size):
super().__init__()
self.interface = interface
self.interface._impl = self
# self.set_title(title)

@property
def interface(self):
return self._interface()

@interface.setter
def interface(self, value):
self._interface = weakref.ref(value)

def set_app(self, app):
self.app = app
native_parent = app.native.findViewById(R__id.content)
Expand Down
1 change: 1 addition & 0 deletions changes/2058.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Window and MainWindow now have 100% test coverage, and complete API documentation.
1 change: 1 addition & 0 deletions changes/2058.removal.1.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Windows no longer need to be explicitly added to the app's window list. When a window is shown, it will be automatically added to the windows for the currently running app.
1 change: 1 addition & 0 deletions changes/2058.removal.2.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The ``multiselect`` argument to Open File and Select Folder dialogs has been renamed ``multiple_select``, for consistency with other widgets that have multiple selection capability.
1 change: 1 addition & 0 deletions changes/2058.removal.3.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``Window.resizeable`` and ``Window.closeable`` have been renamed ``Window.resizable`` and ``Window.closable``, to adhere to US spelling conventions.
82 changes: 68 additions & 14 deletions cocoa/src/toga_cocoa/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import inspect
import os
import sys
import weakref
from urllib.parse import unquote, urlparse

from rubicon.objc.eventloop import CocoaLifecycle, EventLoopPolicy
Expand Down Expand Up @@ -112,13 +113,20 @@ class App:

def __init__(self, interface):
self.interface = interface
self.interface._impl = self

self._cursor_visible = True

asyncio.set_event_loop_policy(EventLoopPolicy())
self.loop = asyncio.new_event_loop()

@property
def interface(self):
return self._interface()

@interface.setter
def interface(self, value):
self._interface = weakref.ref(value)

def create(self):
self.native = NSApplication.sharedApplication
self.native.setActivationPolicy(NSApplicationActivationPolicyRegular)
Expand All @@ -135,12 +143,21 @@ def create(self):
self.appDelegate.native = self.native
self.native.setDelegate_(self.appDelegate)

formal_name = self.interface.formal_name
self._create_app_commands()

# Call user code to populate the main window
self.interface._startup()

# Create the lookup table of menu items,
# then force the creation of the menus.
self.create_menus()

def _create_app_commands(self):
formal_name = self.interface.formal_name
self.interface.commands.add(
# ---- App menu -----------------------------------
toga.Command(
lambda _: self.interface.about(),
self._menu_about,
"About " + formal_name,
group=toga.Group.APP,
),
Expand Down Expand Up @@ -176,12 +193,35 @@ def create(self):
),
# Quit should always be the last item, in a section on its own
toga.Command(
lambda _: self.interface.exit(),
self._menu_exit,
"Quit " + formal_name,
shortcut=toga.Key.MOD_1 + "q",
group=toga.Group.APP,
section=sys.maxsize,
),
# ---- File menu ----------------------------------
# This is a bit of an oddity. Safari has 2 distinct "Close Window" and
# "Close All Windows" menu items (partially to differentiate from "Close
# Tab"). Most other Apple HIG apps have a "Close" item that becomes
# "Close All" when you press Option (MOD_2). That behavior isn't something
# we're currently set up to implement, so we live with a separate menu item
# for now.
toga.Command(
self._menu_close_window,
"Close Window",
shortcut=toga.Key.MOD_1 + "W",
group=toga.Group.FILE,
order=1,
section=50,
),
toga.Command(
self._menu_close_all_windows,
"Close All Windows",
shortcut=toga.Key.MOD_2 + toga.Key.MOD_1 + "W",
group=toga.Group.FILE,
order=2,
section=50,
),
# ---- Edit menu ----------------------------------
toga.Command(
NativeHandler(SEL("undo:")),
Expand Down Expand Up @@ -244,26 +284,39 @@ def create(self):
section=10,
order=60,
),
# ---- Window menu ----------------------------------
toga.Command(
self._menu_minimize,
"Minimize",
shortcut=toga.Key.MOD_1 + "m",
group=toga.Group.WINDOW,
),
# ---- Help menu ----------------------------------
toga.Command(
lambda _: self.interface.visit_homepage(),
lambda _, **kwargs: self.interface.visit_homepage(),
"Visit homepage",
enabled=self.interface.home_page is not None,
group=toga.Group.HELP,
),
)
self._create_app_commands()

# Call user code to populate the main window
self.interface._startup()
def _menu_about(self, app, **kwargs):
self.interface.about()

# Create the lookup table of menu items,
# then force the creation of the menus.
self.create_menus()
def _menu_exit(self, app, **kwargs):
self.interface.exit()

def _create_app_commands(self):
# No extra commands
pass
def _menu_close_window(self, app, **kwargs):
if self.interface.current_window:
self.interface.current_window._impl.native.performClose(None)

def _menu_close_all_windows(self, app, **kwargs):
for window in self.interface.windows:
window._impl.native.performClose(None)

def _menu_minimize(self, app, **kwargs):
if self.interface.current_window:
self.interface.current_window._impl.native.miniaturize(None)

def create_menus(self):
# Recreate the menu
Expand Down Expand Up @@ -427,6 +480,7 @@ def select_file(self, **kwargs):

class DocumentApp(App):
def _create_app_commands(self):
super()._create_app_commands()
self.interface.commands.add(
toga.Command(
lambda _: self.select_file(),
Expand Down
13 changes: 7 additions & 6 deletions cocoa/src/toga_cocoa/container.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import weakref

from rubicon.objc import objc_method

from .libs import (
Expand Down Expand Up @@ -32,7 +34,7 @@ def __init__(
min_width=100,
min_height=100,
layout_native=None,
on_refresh=None,
parent=None,
):
"""A container for layouts.

Expand All @@ -45,11 +47,11 @@ def __init__(
itself; however, for widgets like ScrollContainer where the layout needs to
be computed based on a different size to what will be rendered, the source
of the size can be different.
:param on_refresh: The callback to be notified when this container's layout is
refreshed.
:param parent: The parent of this container; this is the object that will be
notified when this container's layout is refreshed.
"""
self._content = None
self.on_refresh = on_refresh
self.parent = weakref.ref(parent)

self.native = TogaView.alloc().init()
self.layout_native = self.native if layout_native is None else layout_native
Expand Down Expand Up @@ -103,8 +105,7 @@ def content(self, widget):
widget.container = self

def refreshed(self):
if self.on_refresh:
self.on_refresh(self)
self.parent().content_refreshed(self)

@property
def width(self):
Expand Down
Loading