From 090044c35da2ded1b7407efd74ed0a1663bd694f Mon Sep 17 00:00:00 2001 From: S4NKALP Date: Sun, 9 Aug 2026 01:26:01 +0545 Subject: [PATCH] fix(globalmenu): rework menu discovery for fresh apps - probe once per app instead of per matched bus service - skip unique :1.x names, which can be stale and never answer DBus - give fresh apps a 6s probe budget so late-exporting menubars are caught - fix app-age computed as inf when unregistered unique names poison the first-seen min, disabling the fresh path entirely - cache no-menu per app/service so re-focus returns instantly - skip oversized action menus and truncate top-level menu to 32 items - parse org.gtk.Actions state via explicit child walk (av unwrap was fragile on apps like Inkscape) --- src/window/globalmenu/gtkmenu.py | 115 ++++---- src/window/globalmenu/service.py | 343 +++++++++++++++++----- src/window/panel/components/globalmenu.py | 9 + 3 files changed, 336 insertions(+), 131 deletions(-) diff --git a/src/window/globalmenu/gtkmenu.py b/src/window/globalmenu/gtkmenu.py index d2e34e3..488f65c 100644 --- a/src/window/globalmenu/gtkmenu.py +++ b/src/window/globalmenu/gtkmenu.py @@ -122,6 +122,27 @@ class _ActionInfo: self.state_type = state_type +def _action_state(state_av) -> tuple[Optional[Any], str]: + """Extract (state, state_type) from an org.gtk.Actions state array. + + ``state_av`` is a GVariant of type ``av``; each element is a variant + wrapping the action's state value. The array is empty for stateless + actions. + """ + if state_av is None or state_av.n_children() == 0: + return None, "" + st = state_av.get_child_value(0) + try: + inner = st.get_variant() + return inner.unpack(), inner.get_type_string() + except Exception: + try: + return st.unpack(), st.get_type_string() + except Exception as e: + logger.debug(f"[gtkmenu] _action_state fallback failed: {e}") + return None, "" + + class GtkMenuClient: __slots__ = ( "_action_cache", @@ -178,25 +199,22 @@ class GtkMenuClient: ) if not res: continue - data = res.get_child_value(0).unpack() - self._action_cache.clear() - for name, (enabled, param_type_v, state_v) in data.items(): - param_type = str(param_type_v) if param_type_v else "" - state = ( - state_v.unpack() - if state_v and state_v.n_children() > 0 - else None - ) - state_type = "" - if state_v and state_v.n_children() > 0: - st = state_v.get_child_value(0) - state_type = st.get_type_string() if st else "" - self._action_cache[name] = _ActionInfo( + data = res.get_child_value(0) + cache = {} + for i in range(data.n_children()): + pair = data.get_child_value(i) + name = pair.get_child_value(0).get_string() + desc = pair.get_child_value(1) + enabled = desc.get_child_value(0).get_boolean() + param_type = desc.get_child_value(1).get_string() + state, state_type = _action_state(desc.get_child_value(2)) + cache[name] = _ActionInfo( enabled=enabled, parameter_type=param_type, state=state, state_type=state_type, ) + self._action_cache = cache return except Exception as e: logger.debug(f"[GtkMenuClient] DescribeAll cache error: {e}") @@ -215,17 +233,9 @@ class GtkMenuClient: path=path, ) if res: - enabled, param_type_v, state_v = res.unpack() - param_type = str(param_type_v) if param_type_v else "" - state = ( - state_v.unpack() - if state_v and state_v.n_children() > 0 - else None - ) - state_type = "" - if state_v and state_v.n_children() > 0: - st = state_v.get_child_value(0) - state_type = st.get_type_string() if st else "" + enabled = res.get_child_value(0).get_boolean() + param_type = res.get_child_value(1).get_string() + state, state_type = _action_state(res.get_child_value(2)) info = _ActionInfo(enabled, param_type, state, state_type) self._action_cache[action_name] = info return info @@ -475,23 +485,22 @@ class ActionMenuClient: ) if not res: return - data = res.get_child_value(0).unpack() - self._action_cache.clear() - for name, (enabled, param_type_v, state_v) in data.items(): - param_type = str(param_type_v) if param_type_v else "" - state = ( - state_v.unpack() if state_v and state_v.n_children() > 0 else None - ) - state_type = "" - if state_v and state_v.n_children() > 0: - st = state_v.get_child_value(0) - state_type = st.get_type_string() if st else "" - self._action_cache[name] = _ActionInfo( + data = res.get_child_value(0) + cache = {} + for i in range(data.n_children()): + pair = data.get_child_value(i) + name = pair.get_child_value(0).get_string() + desc = pair.get_child_value(1) + enabled = desc.get_child_value(0).get_boolean() + param_type = desc.get_child_value(1).get_string() + state, state_type = _action_state(desc.get_child_value(2)) + cache[name] = _ActionInfo( enabled=enabled, parameter_type=param_type, state=state, state_type=state_type, ) + self._action_cache = cache except Exception as e: logger.debug(f"[ActionMenuClient] DescribeAll cache error: {e}") @@ -507,15 +516,9 @@ class ActionMenuClient: GLib.VariantType("(bgav)"), ) if res: - enabled, param_type_v, state_v = res.unpack() - param_type = str(param_type_v) if param_type_v else "" - state = ( - state_v.unpack() if state_v and state_v.n_children() > 0 else None - ) - state_type = "" - if state_v and state_v.n_children() > 0: - st = state_v.get_child_value(0) - state_type = st.get_type_string() if st else "" + enabled = res.get_child_value(0).get_boolean() + param_type = res.get_child_value(1).get_string() + state, state_type = _action_state(res.get_child_value(2)) info = _ActionInfo(enabled, param_type, state, state_type) self._action_cache[action_name] = info return info @@ -624,21 +627,17 @@ class ActionMenuClient: return self._cached_items try: - data = res.get_child_value(0).unpack() + data = res.get_child_value(0) items: List[DBusMenuItem] = [] self._action_map = {} - for i, (name, (enabled, param_type_v, state_v)) in enumerate(data.items()): + for i in range(data.n_children()): + pair = data.get_child_value(i) + name = pair.get_child_value(0).get_string() + desc = pair.get_child_value(1) + enabled = desc.get_child_value(0).get_boolean() + param_type = desc.get_child_value(1).get_string() + state, state_type = _action_state(desc.get_child_value(2)) label = name.replace("-", " ").replace("_", " ").strip().title() - param_type = str(param_type_v) if param_type_v else "" - state = None - state_type = "" - if state_v and state_v.n_children() > 0: - st = state_v.get_child_value(0) - state_type = st.get_type_string() if st else "" - try: - state = st.get_value() - except Exception: - state = state_v.unpack() if state_v else None self._action_cache[name] = _ActionInfo( enabled, param_type, state, state_type ) diff --git a/src/window/globalmenu/service.py b/src/window/globalmenu/service.py index 1728040..ecc1696 100644 --- a/src/window/globalmenu/service.py +++ b/src/window/globalmenu/service.py @@ -23,9 +23,17 @@ from window.globalmenu.registrar import Registrar _NODE_NAME_RE = re.compile(r' str: @@ -161,8 +171,11 @@ class GlobalMenuService(Service): self._service_registry: dict[str, AppServiceInfo] = {} self._pid_to_service: dict[int, str] = {} + self._pid_first_seen: dict[int, float] = {} self._introspection_cache = _IntrospectionCache() + self._no_menu_until: dict[str, float] = {} + self._no_menu_until_app: dict[str, float] = {} self._seq_lock = threading.Lock() self._extraction_seq = 0 @@ -311,6 +324,7 @@ class GlobalMenuService(Service): service_name=service_name, pid=pid, executable=executable, + first_seen=time.monotonic(), ) if pid: self._pid_to_service[pid] = service_name @@ -327,10 +341,14 @@ class GlobalMenuService(Service): same_class = wm_class == self._current_wm_class same_pid = target_pid != 0 and target_pid == self._current_pid + if target_pid: + with self._state_lock: + self._pid_first_seen.setdefault(target_pid, time.monotonic()) + if same_class and same_pid: with self._state_lock: current_menu = self._current_menu - if current_menu is not None: + if current_menu: self.menu_changed(current_menu) return @@ -440,13 +458,12 @@ class GlobalMenuService(Service): self._current_menu = full_menu elif parent_id == 0: self._current_menu = subtree - with self._seq_lock: - self._extraction_seq += 1 - seq = self._extraction_seq + with self._state_lock: + current_wm_class = self._current_wm_class idle_add( self._emit_menu_changed, self._current_menu or subtree, - seq, + current_wm_class, ) def _handle_items_updated( @@ -482,17 +499,12 @@ class GlobalMenuService(Service): items = client.get_layout() or [] - with self._seq_lock: - current_seq = self._extraction_seq - - if seq != current_seq: - return - with self._state_lock: - if wm_class == self._current_wm_class: - self._current_menu = items + if wm_class != self._current_wm_class: + return + self._current_menu = items - idle_add(self._emit_menu_changed, items, current_seq) + idle_add(self._emit_menu_changed, items, wm_class) except Exception as e: logger.error( f"[GlobalMenuService] Error extracting from importer for '{wm_class}': {e}" @@ -534,7 +546,7 @@ class GlobalMenuService(Service): logger.debug(f"[GlobalMenuService] Initial scan error: {e}") def _discover_dbus_client( - self, wm_class: str, target_pid: int = 0 + self, wm_class: str, target_pid: int = 0, fast_only: bool = False ) -> Optional[_MenuClient]: try: if target_pid <= 0: @@ -548,7 +560,7 @@ class GlobalMenuService(Service): strategies = [ self._find_registry, self._find_registrar, - self._find_fallback_scan, + lambda w, p: self._find_fallback_scan(w, p, fast_only=fast_only), ] for strategy in strategies: @@ -588,16 +600,21 @@ class GlobalMenuService(Service): if gtk_client.get_layout(): self._cache_discovery(service, path, pid, wm_class, gtk_client) return gtk_client - self._cache_discovery(service, path, pid, wm_class, client) - return client + logger.debug( + f"[GlobalMenuService] Registrar entry empty: {service} {path}" + ) except Exception as e: if "NameHasNoOwner" in str(e): registrar.unregister_window(window_id) return None def _find_fallback_scan( - self, wm_class: str, target_pid: int + self, wm_class: str, target_pid: int, fast_only: bool = False ) -> Optional[_MenuClient]: + # An app that has been up for a while and showed no menu on a recent + # scan is not going to grow one — skip it to keep re-focus instant. + if wm_class and time.monotonic() < self._no_menu_until_app.get(wm_class, 0): + return None bus = _get_bus() try: res = bus.call_sync( @@ -626,24 +643,117 @@ class GlobalMenuService(Service): if n != "org.freedesktop.DBus" and not n.startswith("org.freedesktop.DBus") ] + # Cheap pass: find every service owned by the app and check the AppMenu + # registrar (fast). A single app usually owns several names (unique + # :1.x names plus well-known ones), so this is collected first and the + # expensive probe below runs only once per app. + matched: list[tuple[str, int]] = [] for svc in services: try: pid = _dbus_get_pid(bus, svc, _DBUS_TIMEOUT_PID) - if not (pid == target_pid or self._is_same_app(pid, target_pid)): - continue + except Exception: + continue + if not (pid == target_pid or self._is_same_app(pid, target_pid)): + continue - registrar = Registrar.get_instance() - reg_entry = registrar.get_menu_for_bus_name(svc) - if reg_entry: - logger.info( - f"[GlobalMenuService] Found Registrar path {reg_entry.object_path} for {svc}" - ) - client = DBusMenuClient(reg_entry.bus_name, reg_entry.object_path) + registrar = Registrar.get_instance() + reg_entry = registrar.get_menu_for_bus_name(svc) + if reg_entry: + logger.info( + f"[GlobalMenuService] Found Registrar path {reg_entry.object_path} for {svc}" + ) + client = DBusMenuClient(reg_entry.bus_name, reg_entry.object_path) + if client.get_layout(): self._cache_discovery( svc, reg_entry.object_path, pid, wm_class, client ) return client + gtk_client = GtkMenuClient(reg_entry.bus_name, reg_entry.object_path) + if gtk_client.get_layout(): + self._cache_discovery( + svc, reg_entry.object_path, pid, wm_class, gtk_client + ) + return gtk_client + matched.append((svc, pid)) + + if not matched: + return None + + # Fast path: GTK apps export their menubar under a predictable object + # root derived from the bus name or the window class (e.g. + # /org/xfce/Thunar/menus/menubar/N). Probe it once against the most + # promising service before crawling the whole object tree (which can + # take many seconds on a busy app). + wm_cls_lower = wm_class.lower() if wm_class else "" + probe_svc = next( + ( + s + for s, _ in matched + if not s.startswith(":") and wm_cls_lower in s.lower() + ), + next((s for s, _ in matched if not s.startswith(":")), matched[0][0]), + ) + # Apps export menu objects lazily (window created, menubar wired up, + # module registered). Introspection results cached from an earlier scan + # can therefore point at dead/empty objects, e.g. + # /org//menus/menubar/0. Drop the cache so this probe sees the + # current object tree. + self._introspection_cache.invalidate(probe_svc) + + wm_path = "/" + wm_class.replace(".", "/") if wm_class else "" + roots = [wm_path] if wm_path and GLib.Variant.is_object_path(wm_path) else [] + svc_path = ( + "/" + probe_svc.replace(".", "/") if not probe_svc.startswith(":") else "" + ) + if svc_path and GLib.Variant.is_object_path(svc_path): + roots.append(svc_path) + roots.append("/org/gtk/Application") + # A freshly-launched app may still be exporting its menubar, so give it + # a longer probe budget; an established app gets the short one. Age is + # taken from the youngest matched service actually in the registry + # (unique names like :1.x are never registered and must not contribute + # a zero), falling back to the pid's first focus if nothing is + # registered yet. + oldest = None + for s, _ in matched: + entry = self._service_registry.get(s) + if entry is None: + continue + if oldest is None or entry.first_seen < oldest: + oldest = entry.first_seen + pid_seen = self._pid_first_seen.get(target_pid) if target_pid else None + if pid_seen and (oldest is None or pid_seen < oldest): + oldest = pid_seen + app_age = time.monotonic() - oldest if oldest else float("inf") + budget = _PROBE_BUDGET_FRESH if app_age < _NO_MENU_APP_AGE else _PROBE_BUDGET + logger.debug( + f"[GlobalMenuService] matched={[s for s, _ in matched]} " + f"probe_svc={probe_svc} age={app_age if app_age != float('inf') else 'inf'}s" + ) + client = self._probe_predictable_menu( + probe_svc, + roots, + bus, + target_pid, + wm_class, + budget=budget, + is_fresh=app_age < _NO_MENU_APP_AGE, + ) + if client: + return client + + if fast_only: + return None + + # Full crawl: per matched service, walk the predictable paths plus a + # bounded DFS, using the persistent introspection cache. Unique names + # (:1.x) are skipped: a pid can own several, some stale/never-answered, + # and menus are exported under well-known names. + for svc, pid in matched: + if svc.startswith(":"): + continue + try: _safe_cls = re.sub(r"[^A-Za-z0-9_]", "", wm_class) if wm_class else "" _safe_cap = _safe_cls.capitalize() if _safe_cls else "" @@ -712,6 +822,12 @@ class GlobalMenuService(Service): if 'interface name="org.gtk.Actions"' in xml: test_client = ActionMenuClient(svc, act_base) if test_items := test_client.get_layout(): + if len(test_items) > _MAX_ACTION_MENU_ITEMS: + logger.warning( + f"[GlobalMenuService] Skipping oversized action menu {svc} " + f"{act_base} ({len(test_items)} items > {_MAX_ACTION_MENU_ITEMS})" + ) + continue logger.info( f"[GlobalMenuService] Action menu: {svc} {act_base} ({len(test_items)} items)" ) @@ -719,15 +835,16 @@ class GlobalMenuService(Service): svc, act_base, pid, wm_class, test_client ) return test_client - - logger.info( - f"[GlobalMenuService] No menu found for {svc} wm={wm_class}" - ) except Exception as e: - logger.warning( - f"[service] pid = _dbus_get_pid(bus, svc, _DBUS_TIMEOUT_PID) failed: {e}" - ) - continue + logger.warning(f"[service] crawl for {svc} failed: {e}") + + # Full discovery found no menu. Remember it per-app so a re-focus does + # not re-probe: a no-menu app has no reason to grow one. Established + # apps get a long skip, freshly-launched ones a short one — their + # menubar may still be exporting. + if wm_class: + ttl = _NO_MENU_TTL if app_age >= _NO_MENU_APP_AGE else _NO_MENU_FRESH_TTL + self._no_menu_until_app[wm_class] = time.monotonic() + ttl return None def _build_action_candidates( @@ -816,6 +933,104 @@ class GlobalMenuService(Service): return fallback_paths, seen_fallback + def _probe_predictable_menu( + self, + svc: str, + roots: list, + bus, + pid: int, + wm_class: str, + budget: float = _PROBE_BUDGET, + is_fresh: bool = False, + ) -> Optional[_MenuClient]: + """Walk shallowly from the app's likely menu roots (derived from both + the bus name and the window class), probing for org.gtk.Menus or + com.canonical.dbusmenu. Menus often live at .../menus/menubar/N where N + varies per window, so children are walked too. Apps that just launched + answer DBus slowly while exporting their menubar, so keep re-probing + until the deadline — the menu appears within a moment of the app + exporting it. Established apps that answer cleanly with no menu + interface at all are considered menu-less and return early; fresh apps + always probe the full budget since their menubar may still be + exporting. Services already known to have no menu are skipped.""" + if time.monotonic() < self._no_menu_until.get(svc, 0): + logger.debug(f"[GlobalMenuService] probe {svc} service-TTL skip") + return None + logger.debug( + f"[GlobalMenuService] probe {svc} budget={budget}s is_fresh={is_fresh}" + ) + deadline = time.monotonic() + budget + while time.monotonic() < deadline: + seen: set[str] = set() + stack = [r for r in roots if r and GLib.Variant.is_object_path(r)] + candidates: dict[str, str] = {} + any_timeout = False + while stack and len(seen) < _MAX_PROBE_PATHS: + path = stack.pop() + if path in seen: + continue + seen.add(path) + try: + res = bus.call_sync( + svc, + path, + "org.freedesktop.DBus.Introspectable", + "Introspect", + None, + GLib.VariantType("(s)"), + Gio.DBusCallFlags.NONE, + _DBUS_TIMEOUT_FAST, + None, + ) + xml = res.get_child_value(0).get_string() if res else "" + except GLib.Error as e: + if "UnknownObject" in (e.message or ""): + continue + any_timeout = True + continue + if "org.gtk.Menus" in xml or "com.canonical.dbusmenu" in xml: + candidates[path] = xml + depth = path.count("/") + if depth <= 6: + for child in _NODE_NAME_RE.findall(xml)[:6]: + stack.append(f"{path}/{child}") + for path, xml in candidates.items(): + if "com.canonical.dbusmenu" in xml: + logger.info( + f"[GlobalMenuService] DBusMenu found (fast): {svc} {path}" + ) + client = DBusMenuClient(svc, path) + self._cache_discovery(svc, path, pid, wm_class, client) + return client + if "org.gtk.Menus" in xml: + test_client = GtkMenuClient(svc, path) + test_items = test_client.get_layout() + if test_items: + logger.info( + f"[GlobalMenuService] GTK menu (fast): {svc} {path} " + f"({len(test_items)} items)" + ) + self._cache_discovery(svc, path, pid, wm_class, test_client) + return test_client + # An established app that answered every probe with no menu + # interface at all is responsive and simply has no menu — stop + # early instead of burning the whole budget. Anything else + # (timeouts while the app exports, or a menu interface whose items + # are still appearing, or a freshly-launched app) keeps re-probing + # until the deadline. + if not is_fresh and not candidates and not any_timeout: + logger.debug( + f"[GlobalMenuService] probe {svc} clean-bail " + f"(candidates={len(candidates)} timeouts={any_timeout})" + ) + return None + time.sleep(0.1) + logger.debug(f"[GlobalMenuService] probe {svc} deadline reached") + self._no_menu_until[svc] = time.monotonic() + ( + _NO_MENU_FRESH_TTL if is_fresh else _NO_MENU_TTL + ) + return None + def _cache_discovery( self, service: str, @@ -832,6 +1047,7 @@ class GlobalMenuService(Service): pid=pid, wm_class=wm_class, importer=client, + first_seen=time.monotonic(), ) self._service_registry[service] = entry if pid: @@ -902,6 +1118,11 @@ class GlobalMenuService(Service): return results def _is_same_app(self, pid1: int, pid2: int) -> bool: + """True when both pids belong to the same application. Only the + executable and comm name are compared — a bare parent/child (ppid) + check is deliberately avoided: on a systemd user session every process + is a child of the session manager, which would falsely match + org.freedesktop.systemd1 for every app and bloat the crawl.""" if pid1 == pid2: return True try: @@ -920,20 +1141,6 @@ class GlobalMenuService(Service): logger.warning( f"[service] with open(f'/proc/(pid1)/comm') as f: comm1 = f.read().st... failed: {e}" ) - - def _ppid(p: int) -> int: - try: - with open(f"/proc/{p}/stat") as f: - parts = f.read().split() - return int(parts[3]) if len(parts) > 4 else 0 - except Exception as e: - logger.warning( - f"[service] with open(f'/proc/(p)/stat') as f: parts = f.read().split... failed: {e}" - ) - return 0 - - if _ppid(pid1) == pid2 or _ppid(pid2) == pid1: - return True except (OSError, PermissionError, FileNotFoundError) as e: logger.warning( f"[service] exe1 = os.path.realpath(f'/proc/(pid1)/exe') failed: {e}" @@ -944,15 +1151,12 @@ class GlobalMenuService(Service): self, wm_class: str, seq: int, target_pid: int = 0, inflight_key: str = "" ): try: - with self._seq_lock: - if seq != self._extraction_seq: - return + client = self._discover_dbus_client(wm_class, target_pid, fast_only=True) + items = client.get_layout() if client else [] - client = self._discover_dbus_client(wm_class, target_pid) - items = [] - - if client: - items = client.get_layout() + if not items: + client = self._discover_dbus_client(wm_class, target_pid) + items = client.get_layout() if client else [] if items: logger.info( @@ -961,17 +1165,12 @@ class GlobalMenuService(Service): else: logger.info(f"[GlobalMenuService] No DBus menu found for '{wm_class}'") - with self._seq_lock: - current_seq = self._extraction_seq - - if seq != current_seq: - return - with self._state_lock: - if wm_class == self._current_wm_class: - self._current_menu = items + if wm_class != self._current_wm_class: + return + self._current_menu = items - idle_add(self._emit_menu_changed, items, current_seq) + idle_add(self._emit_menu_changed, items, wm_class) except Exception as e: logger.error( @@ -982,12 +1181,10 @@ class GlobalMenuService(Service): with self._inflight_lock: self._extracting_in_flight.discard(inflight_key) - def _emit_menu_changed(self, items: list[DBusMenuItem], seq: int = 0): - if seq: - with self._seq_lock: - if seq != self._extraction_seq: - return False + def _emit_menu_changed(self, items: list[DBusMenuItem], wm_class: str = ""): with self._state_lock: + if wm_class and wm_class != self._current_wm_class: + return False self._current_menu = items self.menu_changed(items) return False diff --git a/src/window/panel/components/globalmenu.py b/src/window/panel/components/globalmenu.py index 338d099..4e8fbbd 100644 --- a/src/window/panel/components/globalmenu.py +++ b/src/window/panel/components/globalmenu.py @@ -207,6 +207,8 @@ def _get_firefox_bookmarks(pid, folder_label): TRUNCATE_MENU_AT = 25 +MAX_GLOBAL_MENU_ITEMS = 32 + def _truncate_children(children): if len(children) <= TRUNCATE_MENU_AT: @@ -418,6 +420,13 @@ class GlobalMenuDropdowns: item for item in menu_items if item.label and getattr(item, "visible", True) ] + if len(top_level) > MAX_GLOBAL_MENU_ITEMS: + logger.warning( + f"[GlobalMenu] truncating top-level menu to {MAX_GLOBAL_MENU_ITEMS} " + f"items (from {len(top_level)})" + ) + top_level = top_level[:MAX_GLOBAL_MENU_ITEMS] + if not top_level: self.all_menu_buttons = [self.global_menu_button_title] else: