Less runtime work in Style accessors.

* Remove redundant computations from inside TameStyle accessors.
* Don't construct a URI rewriter function inside every call to
  sanitizeStyleProperty.
* Unify URI rewriting for domicile.cssUri and sanitizeStyleProperty.

Fixes <https://code.google.com/p/google-caja/issues/detail?id=1793>.

@r5490

[MikeSamuel, 2013-07-13 00:19:32]

https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
src/com/google/caja/plugin/domado.js:6556:
allCssProperties.forEachCanonical(function(stylePropertyName) {
Is loop is now including non-canon properties?

[kpreid2, 2013-07-13 01:08:02]

https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
src/com/google/caja/plugin/domado.js:1950: "CSS_PROP": prop
Oh, a note of caution: sanitizeStyleProperty used to close over its
cssPropertyName parameter when invoking uriRewrite (line 1965 in base), but now
it uses the one coming back from sanitizeCssProperty (this line here). I looked
at sanitizeCssProperty and I think those should be identical, but I'd appreciate
a confirmation that that's a valid refactoring.

https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
src/com/google/caja/plugin/domado.js:6556:
allCssProperties.forEachCanonical(function(stylePropertyName) {
On 2013/07/13 00:19:32, MikeSamuel wrote:
> Is loop is now including non-canon properties?

I don't understand the question. I also don't really understand this code (and
in particular I haven't looked into what a non-canonical property is), but I
attempted to preserve the existing behavior exactly. As far as I saw based on
reading CssPropertiesCollection, forEachCanonical will return exactly the same
set of properties as the previous isCanonicalProp test allowed. So, this does
not add any more properties to the exposed set.

So, if there's a bug here (are there supposed to be accessors for non-canonical
properties?) then (a) it's existed since the Domita-to-Domado transition and (b)
it wasn't caught by the tests (I already ran tests on this change).

If there is a bug, please file an issue for it and I'll look into it as a
separate change (to keep this a pure refactoring).

[MikeSamuel, 2013-07-14 05:08:52]

LGTM

2013/7/12  <kpreid.switchb.org@gmail.com>:
>
>
https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
> File src/com/google/caja/plugin/domado.js (right):
>
>
https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
> src/com/google/caja/plugin/domado.js:1950: "CSS_PROP": prop
> Oh, a note of caution: sanitizeStyleProperty used to close over its
> cssPropertyName parameter when invoking uriRewrite (line 1965 in base),
> but now it uses the one coming back from sanitizeCssProperty (this line
> here). I looked at sanitizeCssProperty and I think those should be
> identical, but I'd appreciate a confirmation that that's a valid
> refactoring.
>
>
>
https://codereview.appspot.com/11231043/diff/1/src/com/google/caja/plugin/dom...
> src/com/google/caja/plugin/domado.js:6556:
> allCssProperties.forEachCanonical(function(stylePropertyName) {
> On 2013/07/13 00:19:32, MikeSamuel wrote:
>>
>> Is loop is now including non-canon properties?
>
>
> I don't understand the question. I also don't really understand this
> code (and in particular I haven't looked into what a non-canonical
> property is), but I attempted to preserve the existing behavior exactly.
> As far as I saw based on reading CssPropertiesCollection,
> forEachCanonical will return exactly the same set of properties as the
> previous isCanonicalProp test allowed. So, this does not add any more
> properties to the exposed set.
>
> So, if there's a bug here (are there supposed to be accessors for
> non-canonical properties?) then (a) it's existed since the
> Domita-to-Domado transition and (b) it wasn't caught by the tests (I
> already ran tests on this change).
>
> If there is a bug, please file an issue for it and I'll look into it as
> a separate change (to keep this a pure refactoring).
>
> https://codereview.appspot.com/11231043/