Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Web Inspector: List of breakpoints in Sources tab disappears when ins…
…pector reloads rdar://123641994 https://bugs.webkit.org/show_bug.cgi?id=272098 Reviewed by Devin Rousso. The reported bug can be exposed one of three ways, that the list of breakpoints shown in the source sidebar disappears: (1) When you reload the webpage while the inspector is open, (2) When you navigate to a different page and come back while the inspector is open, and (3) When you close and reopen the inspector. This fix addresses the three ways. Here's how: - For way (1), this happens because: a. The source sidebar has the logic to prune "stale" tree elements, which are elements that point to resources that no longer exists in the current frame. Reloading the webpage causes the main resource to change, which triggers this pruning logic. (Source: https://github.com/WebKit/WebKit/blob/c927c492d5b93193bc5cae300f1ca39eb36cd74c/Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js#L507-L508) b. When adding a breakpoint to show, the source sidebar first checks if there already exists a tree element representing the breakpoint. So, when the page reloads and its breakpoints are added (again), they don't actually get new corresponding tree elements created (Source: https://github.com/WebKit/WebKit/blob/c927c492d5b93193bc5cae300f1ca39eb36cd74c/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js#L1407-L1408). c. The above two operations are in a race. If (a) occurs after (b), the _parents_ of the existing breakpoints will get pruned, since they now point to removed resources from the old webpage and are considered stale. Now, the reason (a) may happen after (b) is because NavigationSidebarPanel uses setTimeout to coalesce potentially-multiple calls of pruneStaleResourceTreeElements into just one, and setTimeout may put the call behind schedule. (Source: https://github.com/WebKit/WebKit/blob/27862391346cb4703757a860478d07f255cf5b57/Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js#L645-L649) So the fix is to remove the coalescence/buffer for calling pruneStaleResourceTreeElements and always allow it to be called. This does mean we're potentially pruning many times in a short time, but since pruning is only done for the top-level elements in the tree outlines, there shouldn't be a significant number of them in most cases. (Pruning nested tree elements when resources are sorted By Path is done by the code here: https://github.com/WebKit/WebKit/blob/bd841a09536799b70842b7e0a7e84890e9853fce/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js#L1073-L1077) - For way (2), this happens because, when updating the source code resource for a breakpoint, which can happen as a result of loading a page a second time, that update is skipped unless either the old resource or the new resource is null. (Source: https://github.com/WebKit/WebKit/blob/c927c492d5b93193bc5cae300f1ca39eb36cd74c/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js#L1668-L1669) The fix is to make it always allow breakpoints' resources to be updated, since when reloading the same page the resources will have new object references in the frame, and those have no reason to be blocked from being updated. - For way (3), this happens because: a. When the inspector is opened, the DebuggerManager loads the list of saved breakpoints from object stores. The manager tries to show them in the sidebar by dispatching the BreakpointAdded events, but they fail to get added because the breakpoints don't have their linked source code resources yet. (Source: https://github.com/WebKit/WebKit/blob/c927c492d5b93193bc5cae300f1ca39eb36cd74c/Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js#L128-L135) b. When a resource gets loaded, it links all loaded breakpoints that belong to this resource and show them in the sidebar. The breakpoints now succeed to show up because they first get linked to their resource. (Source: https://github.com/WebKit/WebKit/blob/c927c492d5b93193bc5cae300f1ca39eb36cd74c/Source/WebInspectorUI/UserInterface/Views/SourcesNavigationSidebarPanel.js#L1187-L1188) c. The above two operations are in a race. If (a) occurs after (b), then during (b) there are no breakpoints loaded to link, and during (a) the loaded breakpoints can't get shown without linked resources. So the fix is, during (b), we also record the resource by its `contentIdentifier`, so in case (a) happens afterwards, we can look up the loaded resource to link to the loaded breakpoints so they can get added. * Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js: (WI.NavigationSidebarPanel): (WI.NavigationSidebarPanel.prototype.closed): (WI.NavigationSidebarPanel.prototype.pruneStaleResourceTreeElements): (WI.NavigationSidebarPanel.prototype._checkOutlinesForPendingViewStateCookie): (WI.NavigationSidebarPanel.prototype._checkForStaleResourcesIfNeeded): Deleted. (WI.NavigationSidebarPanel.prototype._checkForStaleResources): Deleted. - Remove the logic for coalescing multiple calls into pruneStaleResourceTreeElements. Pruning needs to happen immediately when conditions are met to prevent stale tree elements from being discovered by incoming breakpoints which will be removed by a later call to prune. * Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js: (WI.DebuggerManager.prototype._associateBreakpointsWithSourceCode): * Source/WebInspectorUI/UserInterface/Models/SourceCodeLocation.js: (WI.SourceCodeLocation.prototype.setSourceCode): - Always allow breakpoints' linked-to resource to be updated since it might just be a newer reference to the same resource. - Move the `console.assert` into `setSourceCode` instead since it makes more sense being there instead. * Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js: (WI.DebuggerManager.prototype.breakpointsForSourceCode): - If existing breakpoints haven't been loaded yet, record the resource by its `contentIdentifier` for later linking the loaded breakpoints. * Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js: (WI.DebuggerManager.prototype.constructor): - When a breakpoint is loaded, see if its resource has already been loaded to be linked. Canonical link: https://commits.webkit.org/279884@main
- Loading branch information