DevTools: Implement styles toggle:...

DevTools: Implement styles toggle:
1. Split inject.js into injected object and the dispatch.
2. Surround utility function call with try/catch, pass potential exception all way to the client.
3. Fix user and user agent scripts detection
4. Introduce toggleStyle
5. Remove __defineGetter__ from the styles code: make things simple
6. Beautify code.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=14839

[yurys, 2009-04-29 12:29:05]

http://codereview.chromium.org/99184/diff/24/27
File webkit/glue/devtools/js/dom_agent.js (right):

http://codereview.chromium.org/99184/diff/24/27#newcode270
Line 270: this.computedStyle_ = this.makeStyle_(computedStyle);
init this property in the constructor?

http://codereview.chromium.org/99184/diff/24/27#newcode271
Line 271: this.style = this.makeStyle_(inlineStyle);
ditto

http://codereview.chromium.org/99184/diff/24/27#newcode873
Line 873: * @param {boolean} enabled True if style should be enabled.
too many @params

http://codereview.chromium.org/99184/diff/24/27#newcode890
Line 890: name]));
wrong alignment

http://codereview.chromium.org/99184/diff/24/26
File webkit/glue/devtools/js/inject.js (right):

http://codereview.chromium.org/99184/diff/24/26#newcode221
Line 221: devtools.Injected.prototype.toggleNodeStyle = function(node, styleId,
enabled, name) {
long line

http://codereview.chromium.org/99184/diff/24/26#newcode228
Line 228: if (!style.__disabledPropertyValues ||
!style.__disabledPropertyPriorities) {
long line

http://codereview.chromium.org/99184/diff/24/26#newcode229
Line 229: style.__disabledProperties = {};
why not use mapping name->property data?

http://codereview.chromium.org/99184/diff/24/26#newcode244
Line 244: style.__disabledProperties[name] = true;
it seems that this line should go in both branches

http://codereview.chromium.org/99184/diff/24/34
File webkit/glue/devtools/js/inject_dispatch.js (right):

http://codereview.chromium.org/99184/diff/24/34#newcode4
Line 4: 
@fileoverview is missing

http://codereview.chromium.org/99184/diff/24/34#newcode29
Line 29: }
shouldn't we return here?

[yurys, 2009-04-29 12:29:21]

LGTM

[pfeldman, 2009-04-29 12:39:26]

http://codereview.chromium.org/99184/diff/24/27
File webkit/glue/devtools/js/dom_agent.js (right):

http://codereview.chromium.org/99184/diff/24/27#newcode270
Line 270: this.computedStyle_ = this.makeStyle_(computedStyle);
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> init this property in the constructor?

Done.

http://codereview.chromium.org/99184/diff/24/27#newcode271
Line 271: this.style = this.makeStyle_(inlineStyle);
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> ditto

Done.

http://codereview.chromium.org/99184/diff/24/27#newcode873
Line 873: * @param {boolean} enabled True if style should be enabled.
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> too many @params

Done.

http://codereview.chromium.org/99184/diff/24/27#newcode890
Line 890: name]));
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> wrong alignment

Done.

http://codereview.chromium.org/99184/diff/24/26
File webkit/glue/devtools/js/inject.js (right):

http://codereview.chromium.org/99184/diff/24/26#newcode221
Line 221: devtools.Injected.prototype.toggleNodeStyle = function(node, styleId,
enabled, name) {
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> long line

Done.

http://codereview.chromium.org/99184/diff/24/26#newcode228
Line 228: if (!style.__disabledPropertyValues ||
!style.__disabledPropertyPriorities) {
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> long line

Done.

http://codereview.chromium.org/99184/diff/24/26#newcode229
Line 229: style.__disabledProperties = {};
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> why not use mapping name->property data?

Taken from webkit - will ease merging...

http://codereview.chromium.org/99184/diff/24/26#newcode244
Line 244: style.__disabledProperties[name] = true;
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> it seems that this line should go in both branches

Nope. Taken from webkit.

http://codereview.chromium.org/99184/diff/24/34
File webkit/glue/devtools/js/inject_dispatch.js (right):

http://codereview.chromium.org/99184/diff/24/34#newcode4
Line 4: 
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> @fileoverview is missing

Done.

http://codereview.chromium.org/99184/diff/24/34#newcode29
Line 29: }
On 2009/04/29 12:29:06, Yury Semikhatsky wrote:
> shouldn't we return here?

Done.