Bug 148143

Summary:

Web Inspector: Modernize CSSStyleManager

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bburg, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

DoNotImportToRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed Fix	bburg: review+

Description Joseph Pecoraro 2015-08-18 14:32:10 PDT

* SUMMARY
Modernize CSSStyleManager.

  - Eliminate `delete`
  - Use Map instead of Object for a Map.

Comment 1 Joseph Pecoraro 2015-08-18 14:34:48 PDT

Created attachment 259298 [details]
[PATCH] Proposed Fix

Comment 2 BJ Burg 2015-08-18 15:01:08 PDT

Comment on attachment 259298 [details]
[PATCH] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Controllers/CSSStyleManager.js:-227
> -        function syleSheetsFetched()

Ouch!

> Source/WebInspectorUI/UserInterface/Controllers/CSSStyleManager.js:233
>          var key = this._frameURLMapKey(frame, url);

Can we move this above the callback and make it `let`?

> Source/WebInspectorUI/UserInterface/Controllers/CSSStyleManager.js:236
> +        var styleSheet = this._styleSheetFrameURLMap.get(key) || this._styleSheetFrameURLMap.get(url) || null;

let?

> Source/WebInspectorUI/UserInterface/Controllers/CSSStyleManager.js:347
> +            this._ignoreResourceContentDidChangeEventForResource = null;

Why null?

Comment 3 Joseph Pecoraro 2015-08-18 15:08:36 PDT

> > Source/WebInspectorUI/UserInterface/Controllers/CSSStyleManager.js:347
> > +            this._ignoreResourceContentDidChangeEventForResource = null;
> 
> Why null?

I find `null` clearer here. It better signifies our intent for that property, "an object type goes here", better than undefined.

Comment 4 Joseph Pecoraro 2015-08-18 15:12:19 PDT

http://trac.webkit.org/changeset/188601