Bug 51332 - Web Inspector: Make CSSCompletions a real class rather than singleton
Summary: Web Inspector: Make CSSCompletions a real class rather than singleton
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks: 41414
  Show dependency tree / graph
 
Reported: 2010-12-20 07:31 PST by Alexander Pavlov (apavlov)
Modified: 2010-12-20 09:28 PST (History)
10 users (show)

See Also:

Attachments
[PATCH] Suggested solution (18.02 KB, patch)
2010-12-20 08:23 PST, Alexander Pavlov (apavlov) 		no flags Details | Formatted Diff | Diff
[PATCH] Comments addressed (17.89 KB, patch)
2010-12-20 08:47 PST, Alexander Pavlov (apavlov) 		yurys: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2010-12-20 07:31:14 PST 
This refactoring is necessary to implement bug 41414 and looks like a task of its own.
Comment 1 Alexander Pavlov (apavlov) 2010-12-20 08:23:23 PST 
Created attachment 77005 [details]
[PATCH] Suggested solution
Comment 2 Yury Semikhatsky 2010-12-20 08:35:49 PST 
Comment on attachment 77005 [details]
[PATCH] Suggested solution

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

> WebCore/inspector/front-end/CSSCompletions.js:133
> +WebInspector.CSSCompletions.prototype.__proto__ = Array.prototype;

What's the reason for this class to be a descendant of Array? Can you change it to keep an array of properties in a private field instead? I would also recommend passing array of property names to the CSSCompletion constructor.

> WebCore/inspector/front-end/inspector.js:618
> +    InspectorBackend.getSupportedCSSProperties(WebInspector.cssNameCompletions._load.bind(WebInspector.cssNameCompletions));

Method _load should be public, also it would be better to instantiate CSSCompletions only when the property names are available.
Comment 3 Alexander Pavlov (apavlov) 2010-12-20 08:47:44 PST 
Created attachment 77008 [details]
[PATCH] Comments addressed
Comment 4 Yury Semikhatsky 2010-12-20 08:52:04 PST 
Comment on attachment 77008 [details]
[PATCH] Comments addressed

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

> WebCore/inspector/front-end/CSSCompletions.js:36
> +    for (var i = 0; i < values.length; ++i)

Just use this._values = values.slice()
Comment 5 Alexander Pavlov (apavlov) 2010-12-20 09:28:04 PST 
Landed with the suggested change.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/inspector/front-end/CSSCompletions.js
        M       WebCore/inspector/front-end/SourceCSSTokenizer.js
        M       WebCore/inspector/front-end/SourceCSSTokenizer.re2js
        M       WebCore/inspector/front-end/StylesSidebarPane.js
        M       WebCore/inspector/front-end/inspector.js
Committed r74352
Comment 6 Joseph Pecoraro 2010-12-20 09:28:34 PST 
Thanks for making this separate!