Bug 151581 - Web Inspector: save Inspector's breakpoints to localStorage whenever they are modified
Summary: Web Inspector: save Inspector's breakpoints to localStorage whenever they are...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Brian Burg
URL:
Keywords: InRadar
: 129715 (view as bug list)
Depends on:
Blocks: InspectorDebug 151583
  Show dependency treegraph
 
Reported: 2015-11-23 22:38 PST by Brian Burg
Modified: 2015-12-01 17:15 PST (History)
7 users (show)

See Also:


Attachments
Proposed Fix (5.09 KB, patch)
2015-11-23 22:44 PST, Brian Burg
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian 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 Brian Burg 2015-11-23 22:44:40 PST
Created attachment 266122 [details]
Proposed Fix
Comment 3 Brian 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 Brian 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 Brian Burg 2015-11-25 12:54:00 PST
Committed r192771: <http://trac.webkit.org/changeset/192771>