Merge CSSSVGDocumentValue with CSSURIValue

Merge CSSSVGDocumentValue with CSSURIValue

These two CSSValue classes are used to represent the same semantic,
which is a reference to an element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get()s.

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality and
state.

BUG=470608, 405315
Committed: https://crrev.com/496a81df6c42d2672f7d61b0c36dc31723155600
Cr-Commit-Position: refs/heads/master@{#407126}

[fs, 2016-07-20 21:35:01]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[fs, 2016-07-20 21:35:39]

Description was changed from

==========
Merge CSSSVGDocumentValue with CSSURIValue

These two CSS*Value class are used to represent the same semantics,
which is a reference to element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get().

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality (and
state.)

BUG=470608, 405315
==========

to

==========
Merge CSSSVGDocumentValue with CSSURIValue

These two CSS*Value class are used to represent the same semantics,
which is a reference to element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get()s.

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality (and
state.)

BUG=470608, 405315
==========

[commit-bot: I haz the power, 2016-07-20 21:35:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2016-07-20 21:41:30]

Description was changed from

==========
Merge CSSSVGDocumentValue with CSSURIValue

These two CSS*Value class are used to represent the same semantics,
which is a reference to element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get()s.

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality (and
state.)

BUG=470608, 405315
==========

to

==========
Merge CSSSVGDocumentValue with CSSURIValue

These two CSSValue classes are used to represent the same semantic,
which is a reference to an element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get()s.

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality and
state.

BUG=470608, 405315
==========

[fs, 2016-07-20 22:23:24]

fs@opera.com changed reviewers:
+ meade@chromium.org, timloh@chromium.org

[commit-bot: I haz the power, 2016-07-20 23:14:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-20 23:14:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Timothy Loh, 2016-07-21 04:01:45]

https://codereview.chromium.org/2165833006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSVGDocumentValue.cpp (left):

https://codereview.chromium.org/2165833006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSVGDocumentValue.cpp:63: return
serializeString(m_url);
Probably you want to update consumeFilter and createFilterOperations to not wrap
this in an extra CSSFunctionValue

[fs, 2016-07-21 08:39:16]

https://codereview.chromium.org/2165833006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSVGDocumentValue.cpp (left):

https://codereview.chromium.org/2165833006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSVGDocumentValue.cpp:63: return
serializeString(m_url);
On 2016/07/21 at 04:01:45, Timothy Loh wrote:
> Probably you want to update consumeFilter and createFilterOperations to not
wrap this in an extra CSSFunctionValue

Ouch, missed that detail... I guess it's either unwrap for url(...) in 'filter'
or wrap for the other cases. I'll try for the former, but I'd be interested to
hear if there's a preference one way or the other ("functions" are not
consistently represented as such.)

[fs, 2016-07-21 11:10:20]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 11:10:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 12:45:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-21 12:45:58]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[fs, 2016-07-21 12:50:38]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 12:50:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2016-07-21 13:42:08]

https://codereview.chromium.org/2165833006/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSVGDocumentValue.cpp (left):

https://codereview.chromium.org/2165833006/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSVGDocumentValue.cpp:63: return
serializeString(m_url);
On 2016/07/21 at 08:39:16, fs wrote:
> On 2016/07/21 at 04:01:45, Timothy Loh wrote:
> > Probably you want to update consumeFilter and createFilterOperations to not
wrap this in an extra CSSFunctionValue
> 
> Ouch, missed that detail... I guess it's either unwrap for url(...) in
'filter' or wrap for the other cases. I'll try for the former, but I'd be
interested to hear if there's a preference one way or the other ("functions" are
not consistently represented as such.)

Done. (Had to update FilterInterpolationFunctions too.)

[commit-bot: I haz the power, 2016-07-21 15:13:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-21 15:13:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Timothy Loh, 2016-07-22 00:09:21]

lgtm

IIRC I added CSSFunctionValue in the past because there were a few CSSValue
subclasses for representing different functions without providing anything extra
(like the cached document). So it seems fine to me to have a CSSURIValue if we
need this cache.

[fs, 2016-07-22 09:38:10]

On 2016/07/22 at 00:09:21, timloh wrote:
> lgtm
> 
> IIRC I added CSSFunctionValue in the past because there were a few CSSValue
subclasses for representing different functions without providing anything extra
(like the cached document). So it seems fine to me to have a CSSURIValue if we
need this cache.

Ok, thanks for the explanation! I noticed CSSBasicShape*Value which are mostly
fitting that mold, but maybe the syntactical "shape" (no pun intended) of those
("<foo> at <bar" for instance) makes them more suitable as their own thing (not
obvious though.) As for CSSURIValue, I expect I'll add some more "extra" to it
in the near future.

[fs, 2016-07-22 09:38:24]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-07-22 09:38:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 10:49:08]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-22 10:50:28]

Description was changed from

==========
Merge CSSSVGDocumentValue with CSSURIValue

These two CSSValue classes are used to represent the same semantic,
which is a reference to an element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get()s.

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality and
state.

BUG=470608, 405315
==========

to

==========
Merge CSSSVGDocumentValue with CSSURIValue

These two CSSValue classes are used to represent the same semantic,
which is a reference to an element by means of a "url(...)" function.
CSSSVGDocumentValue carries additional state to be able to handle
references to external documents, by caching a DocumentResource.

Move all the DocumentResource state to CSSURIValue, while also keeping
the naming of the URL string (m_url). Also do some minor cleanup, for
instance by removing "SVG" from the cachedSVGDocument() method name and
dropping some unnecessary .get()s.

This bloats CSSURIValue a bit for the cases where the DocumentResource
is not utilized, but this bloat will be reduced (in relative terms) with
future developments. On the "pro"-side is naturally that we now don't
have to extend two CSSValue classes with additional functionality and
state.

BUG=470608, 405315

Committed: https://crrev.com/496a81df6c42d2672f7d61b0c36dc31723155600
Cr-Commit-Position: refs/heads/master@{#407126}
==========

[commit-bot: I haz the power, 2016-07-22 10:50:29]

Patchset 3 (id:??) landed as
https://crrev.com/496a81df6c42d2672f7d61b0c36dc31723155600
Cr-Commit-Position: refs/heads/master@{#407126}