"Fix Committed" in VCS for stonking, but not uploaded to avoid
interfering with the gnome-shell 51~alpha transition.

** Description changed:

- SUMMARY
+ [ Impact ]
+ 
+ If the default HTTP URL handler is a "hidden" application, every time
+ you lock and unlock the screen a random shell search provider is
+ removed.
+ 
+ Eventually, all search built-in search providers will be removed and
+ searching for installed applications will yield no results.
+ 
+ This is notably the case for Google's official Chrome deb package, which
+ ships two desktop entries: com.google.Chrome.desktop as hidden, and
+ google-chrome.desktop as visible. If the hidden one is set as the
+ default Web Browser, then the bug will happen.
+ 
+ [ Test Plan ]
+ 
+ 0. Install the gnome-shell update, log-out and log-in again (or reboot)
+ 1. Install Google Chrome from https://www.google.com/chrome/
+ 2. Open Google Chrome
+ 3. When prompted, set it as the default browser
+ 4. Lock and unlock the screen 20 times in a row
+ 5. Open the applications menu
+ 6. Verify that searching for an installed application does find it
+ 
+ [ Where problems could occur ]
+ 
+ GNOME Shell provides the desktop UI in Ubuntu Desktop. Issues could
+ manifest anywhere in the desktop. The change is trivial enough to not
+ expect any regressions.
+ 
+ [ Other info -- the full original analysis ]
  
  SearchResultsView._unregisterProvider() does not check the result of
  indexOf():
  
-     _unregisterProvider(provider) {
-         const index = this._providers.indexOf(provider);
-         this._providers.splice(index, 1);
+     _unregisterProvider(provider) {
+         const index = this._providers.indexOf(provider);
+         this._providers.splice(index, 1);
  
-         if (provider.display)
-             provider.display.destroy();
-     }
+         if (provider.display)
+             provider.display.destroy();
+     }
  
  If the passed provider is not in this._providers, indexOf() returns -1 and
  splice(-1, 1) silently removes the LAST provider in the array - an arbitrary,
  unrelated provider. Since SearchController.removeProvider() is a thin wrapper
  around this method, the public API documented for extensions (gjs.guide) is
  affected as well.
  
  This is easy to trigger because _registerProvider() can silently refuse to
  register a provider:
  
-     _registerProvider(provider) {
-         provider.searchInProgress = false;
+     _registerProvider(provider) {
+         provider.searchInProgress = false;
  
-         // Filter out unwanted providers.
-         if (provider.appInfo && 
!this._parentalControlsManager.shouldShowApp(provider.appInfo))
-             return;
+         // Filter out unwanted providers.
+         if (provider.appInfo && 
!this._parentalControlsManager.shouldShowApp(provider.appInfo))
+             return;
  
-         this._providers.push(provider);
-         this._ensureProviderDisplay(provider);
-     }
+         this._providers.push(provider);
+         this._ensureProviderDisplay(provider);
+     }
  
  The caller has no way to know registration was refused, keeps its provider
  reference, and will later call removeProvider()/_unregisterProvider() with a
  provider that was never registered - killing an innocent provider each time.
  
  REAL-WORLD IMPACT (how this was found)
  
  On Ubuntu 26.04, the [email protected] extension registers a
  provider whose appInfo is the xdg default browser. Google Chrome's deb package
  ships com.google.Chrome.desktop with NoDisplay=true (an alias kept for XDG
  portal app-ID matching), and "xdg-settings get default-web-browser" can
  legitimately point at it. In that configuration:
  
  1. On every extension enable (login/unlock), registration is silently refused
-    (shouldShowApp() -> appInfo.should_show() -> false), but the extension
-    keeps its reference.
+    (shouldShowApp() -> appInfo.should_show() -> false), but the extension
+    keeps its reference.
  2. On every extension disable (screen lock/suspend), _unregisterProvider() is
-    called with the never-registered provider -> splice(-1, 1) destroys the
-    last provider in the list.
+    called with the never-registered provider -> splice(-1, 1) destroys the
+    last provider in the list.
  3. Remote providers are resurrected by _reloadRemoteProviders() on
-    installed-changed (e.g. snap refreshes rewriting .desktop files), but the
-    AppSearchProvider ('applications') is only registered once, in the
-    SearchResultsView constructor. Once lock/unlock cycles between two reloads
-    eat through the tail of the list and reach it, application search is dead
-    for the rest of the session - the overview shows "No results" for every
-    app, with nothing in the logs.
+    installed-changed (e.g. snap refreshes rewriting .desktop files), but the
+    AppSearchProvider ('applications') is only registered once, in the
+    SearchResultsView constructor. Once lock/unlock cycles between two reloads
+    eat through the tail of the list and reach it, application search is dead
+    for the rest of the session - the overview shows "No results" for every
+    app, with nothing in the logs.
  
  The stochastic race between suspend/lock cycles and installed-changed reloads
  makes the breakage look random. Only a re-login fixes it. (The extension's
  side of the bug - keeping a stale reference - is filed separately against
  gnome-shell-ubuntu-extensions: <LINK-BUG-EXTENSION>)
  
  MINIMAL REPRODUCER
  
  In Looking Glass (Alt+F2, "lg"), on any session:
  
-     Main.overview.searchController._searchResults._providers.map(p => 
p.id).join(' | ')
-     // note the list, then:
-     Main.overview.searchController.removeProvider({})
-     Main.overview.searchController._searchResults._providers.map(p => 
p.id).join(' | ')
-     // -> the last provider has been silently removed and its display 
destroyed
+     Main.overview.searchController._searchResults._providers.map(p => 
p.id).join(' | ')
+     // note the list, then:
+     Main.overview.searchController.removeProvider({})
+     Main.overview.searchController._searchResults._providers.map(p => 
p.id).join(' | ')
+     // -> the last provider has been silently removed and its display 
destroyed
  
  SUGGESTED FIX
  
-     _unregisterProvider(provider) {
-         const index = this._providers.indexOf(provider);
-         if (index < 0)
-             return; // or log a warning
+     _unregisterProvider(provider) {
+         const index = this._providers.indexOf(provider);
+         if (index < 0)
+             return; // or log a warning
  
-         this._providers.splice(index, 1);
+         this._providers.splice(index, 1);
  
-         if (provider.display)
-             provider.display.destroy();
-     }
+         if (provider.display)
+             provider.display.destroy();
+     }
  
  Additionally, consider making _registerProvider()'s refusal observable (return
  a boolean, or log), so callers don't end up holding references to providers
  that were never registered.

** Changed in: gnome-shell (Ubuntu)
       Status: Triaged => Fix Committed

** Changed in: gnome-shell (Ubuntu)
     Assignee: (unassigned) => Alessandro Astone (aleasto)

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2161808

Title:
  SearchResultsView._unregisterProvider() removes an arbitrary provider
  when called with an unregistered provider (splice(-1, 1))

To manage notifications about this bug go to:
https://bugs.launchpad.net/gnome-shell/+bug/2161808/+subscriptions


-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to