Leverage static `deserialize` functions, as well as `Multimap`s to hold the breakpoints.
<rdar://problem/49236485>
Created attachment 365921 [details] Patch
Comment on attachment 365921 [details] Patch Attachment 365921 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11666184 New failing tests: inspector/debugger/probe-manager-add-remove-actions.html
Created attachment 365930 [details] Archive of layout-test-results from ews104 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 365921 [details] Patch Attachment 365921 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11666514 New failing tests: inspector/debugger/probe-manager-add-remove-actions.html
Created attachment 365932 [details] Archive of layout-test-results from ews101 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 365921 [details] Patch Attachment 365921 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11666990 New failing tests: inspector/debugger/probe-manager-add-remove-actions.html
Created attachment 365937 [details] Archive of layout-test-results from ews117 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 365975 [details] Patch
Comment on attachment 365975 [details] Patch Attachment 365975 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/11675250 New failing tests: animations/resume-after-page-cache.html
Created attachment 365996 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment on attachment 365975 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365975&action=review r=me > Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:223 > + return Array.from(this.breakpointsForSourceCode(sourceCode.sourceMap.originalSourceCode)) > + .filter((breakpoint) => breakpoint.sourceCodeLocation.displaySourceCode === sourceCode); I don't think this is our usual style. $ ack '^\s+\.[^.]' **/*.js -B 1 --ignore External It is vastly more common to keep this on one line or to at least put the function name on the same line: return Array.from(this.breakpointsForSourceCode(sourceCode.sourceMap.originalSourceCode)).filter((breakpoint) => { return breakpoint.sourceCodeLocation.displaySourceCode === sourceCode; }); > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:-33 > - if (sourceCodeLocationOrInfo instanceof WI.SourceCodeLocation) { > - var sourceCode = sourceCodeLocationOrInfo.sourceCode; Some of the oldest ugliest code. Glad to see this cleaned up! > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:41 > + this._sourceCodeLocation = sourceCodeLocation; After this I'd put: this._contentIdentifier = null; It used to be `null` and would otherwise be `undefined` with this patch for special breakpoints. > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:64 > + // Static We can make this an "Import / Export" section and put `deserialize` (or `fromJSON`) and `toJSON` next to each other to it is easy to compare them. > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:70 > + // The 'url' fallback is for transitioning from older frontends and should be removed. Now might be a good time to remove it! > Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:244 > - return newAction; > + return this._actions[index]; Would be easier to avoid the lookup again by storing the new action in a local. Especially given someone could modify this_actions in the event handler. > Source/WebInspectorUI/UserInterface/Models/BreakpointAction.js:39 > + // Static Same here re: "Import / Export" or at least putting it by the toJSON. Also, we don't use the "id" in deserialization, but we presumably do for the protocol? There should probably be a toProtocol() [protocol] instead of toJSON() [serialization].
Created attachment 366435 [details] Patch
Comment on attachment 366435 [details] Patch Clearing flags on attachment: 366435 Committed r243727: <https://trac.webkit.org/changeset/243727>
All reviewed patches have been landed. Closing bug.