Bug 151581

Summary: Web Inspector: save Inspector's breakpoints to localStorage whenever they are modified
Product: WebKit Reporter: BJ Burg <bburg>
Component: Web InspectorAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 147066, 151583    
Attachments:
Description Flags
Proposed Fix timothy: review+

Description BJ Burg 2015-11-23 22:38:19 PST
We try to only save on pagehide, but this event doesn't seem to be sent on fast quit (like Cmd-Q of Safari or Ctrl-C when run from command line). Just save on every modification, since it's probably cheap in most realistic scenarios.

If it turns out to be expensive for some reason, we could coalesce updates using a timer.
Comment 1 Radar WebKit Bug Importer 2015-11-23 22:38:32 PST
<rdar://problem/23653243>
Comment 2 BJ Burg 2015-11-23 22:44:40 PST
Created attachment 266122 [details]
Proposed Fix
Comment 3 BJ Burg 2015-11-23 22:48:52 PST
*** Bug 129715 has been marked as a duplicate of this bug. ***
Comment 4 Timothy Hatcher 2015-11-24 09:36:59 PST
Comment on attachment 266122 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=266122&action=review

> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:877
> +        let savedBreakpoints = this._breakpoints
> +            .filter((breakpoint) => !!breakpoint.url)
> +            .map((breakpoint) => breakpoint.info);

Not a fan of hanging periods.
Comment 5 BJ Burg 2015-11-24 10:58:29 PST
Comment on attachment 266122 [details]
Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=266122&action=review

>> Source/WebInspectorUI/UserInterface/Controllers/DebuggerManager.js:877
>> +            .map((breakpoint) => breakpoint.info);
> 
> Not a fan of hanging periods.

Oh, I know.. I always write it this way then change it back before submitting the patch. It's just so much easier for me to think "oh, a bunch of leading periods, this must be a chain, I'll look up and to the right" than to see the line mysteriously indented (is it a completion handler? a chain? I don't know). But if nobody else likes it (as it seems to be the case) I'll switch it back.
Comment 6 BJ Burg 2015-11-25 12:54:00 PST
Committed r192771: <http://trac.webkit.org/changeset/192771>