[Typed-OM] Add CSSResourceValue

[Typed-OM] Add CSSResourceValue

Spec: https://drafts.css-houdini.org/css-typed-om/#resourcevalue-objects

BUG=545318
Committed: https://crrev.com/f438b039b4f3d4f8875e71e8387ac3ddcbc2fb7e
Cr-Commit-Position: refs/heads/master@{#411020}

[anthonyhkf, 2016-08-01 02:53:57]

anthonyhkf@google.com changed reviewers:
+ meade@chromium.org, shanestephens@google.com

[anthonyhkf, 2016-08-01 23:47:55]

anthonyhkf@google.com changed reviewers:
+ shans@chromium.org
- shanestephens@google.com

[meade_UTC10, 2016-08-02 00:58:13]

meade@chromium.org changed reviewers:
+ dstockwell@chromium.org

[meade_UTC10, 2016-08-02 00:58:14]

lgtm after addressing test comments.

+doug for OWNERS review

https://codereview.chromium.org/2190543004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSResourceValueTest.cpp (right):

https://codereview.chromium.org/2190543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSResourceValueTest.cpp:17:
FakeCSSResourceValue(Resource* resource)
Since you've made CSSResourceValue not store a resource, taking this in here
doesn't make sense.

Best to remove this, FakeResource, and m_resource, and just make
FakeCSSResourceValue take a Resource::Status.

[anthonyhkf, 2016-08-02 01:16:16]

https://codereview.chromium.org/2190543004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSResourceValueTest.cpp (right):

https://codereview.chromium.org/2190543004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSResourceValueTest.cpp:17:
FakeCSSResourceValue(Resource* resource)
On 2016/08/02 00:58:14, Eddy wrote:
> Since you've made CSSResourceValue not store a resource, taking this in here
> doesn't make sense.
> 
> Best to remove this, FakeResource, and m_resource, and just make
> FakeCSSResourceValue take a Resource::Status.

Done.

[meade_UTC10, 2016-08-04 06:55:13]

https://codereview.chromium.org/2190543004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSStyleValue.h (right):

https://codereview.chromium.org/2190543004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSStyleValue.h:30: ResourceStateType,
This should just be ResourceType.

[anthonyhkf, 2016-08-04 07:25:35]

https://codereview.chromium.org/2190543004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/CSSStyleValue.h (right):

https://codereview.chromium.org/2190543004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/CSSStyleValue.h:30: ResourceStateType,
On 2016/08/04 06:55:13, Eddy wrote:
> This should just be ResourceType.

Done.

[meade_UTC10, 2016-08-08 02:48:58]

Ping Doug? :)

[anthonyhkf, 2016-08-08 06:52:24]

The CQ bit was checked by anthonyhkf@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 06:52:43]

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-08-08 07:06:14]

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

[commit-bot: I haz the power, 2016-08-08 07:06:15]

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

[anthonyhkf, 2016-08-08 07:13:52]

https://codereview.chromium.org/2190543004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/CSSResourceValue.h (right):

https://codereview.chromium.org/2190543004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/CSSResourceValue.h:22: const String
state() const
Anyone know what I should return at the end of this function? It has error
"control reaches end of non-void function" for the dry run.

Thanks!

[Timothy Loh, 2016-08-08 07:17:57]

https://codereview.chromium.org/2190543004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/CSSResourceValue.h (right):

https://codereview.chromium.org/2190543004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/CSSResourceValue.h:22: const String
state() const
On 2016/08/08 07:13:52, anthonyhkf wrote:
> Anyone know what I should return at the end of this function? It has error
> "control reaches end of non-void function" for the dry run.
> 
> Thanks!

A couple of options, e.g.

default:
    NOTREACHED();
    return "";

or you can put it at the top

switch (status()) {
default:
    NOTREACHED();
case ...

[anthonyhkf, 2016-08-08 07:42:17]

The CQ bit was checked by anthonyhkf@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 07:42:32]

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

[anthonyhkf, 2016-08-08 07:42:52]

On 2016/08/08 07:17:57, Timothy Loh wrote:
>
https://codereview.chromium.org/2190543004/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/css/cssom/CSSResourceValue.h (right):
> 
>
https://codereview.chromium.org/2190543004/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/css/cssom/CSSResourceValue.h:22: const String
> state() const
> On 2016/08/08 07:13:52, anthonyhkf wrote:
> > Anyone know what I should return at the end of this function? It has error
> > "control reaches end of non-void function" for the dry run.
> > 
> > Thanks!
> 
> A couple of options, e.g.
> 
> default:
>     NOTREACHED();
>     return "";
> 
> or you can put it at the top
> 
> switch (status()) {
> default:
>     NOTREACHED();
> case ...

Thank you :D

[commit-bot: I haz the power, 2016-08-08 09:12:36]

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

[commit-bot: I haz the power, 2016-08-08 09:12:37]

Dry run: This issue passed the CQ dry run.

[dstockwell, 2016-08-10 03:55:17]

lgtm

https://codereview.chromium.org/2190543004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl (right):

https://codereview.chromium.org/2190543004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl:6: "unloaded",
"loading", "loaded", "error"
Does this not generate anything that we can refer to from CSSResourceValue.h?

[meade_UTC10, 2016-08-10 05:00:40]

https://codereview.chromium.org/2190543004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl (right):

https://codereview.chromium.org/2190543004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/cssom/CSSResourceValue.idl:6: "unloaded",
"loading", "loaded", "error"
On 2016/08/10 03:55:17, dstockwell wrote:
> Does this not generate anything that we can refer to from CSSResourceValue.h?

It doesn't seem to - for example the enum LengthType in
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/cssom...
just results in an array of valid values in the generated V8CSSLengthValue:
https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/core/v8/V8C....
I couldn't find any other references to "LengthType" in the generated code.

[anthonyhkf, 2016-08-10 05:02:13]

The CQ bit was checked by anthonyhkf@google.com

[anthonyhkf, 2016-08-10 05:02:14]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2190543004/#ps120001
(title: "Add default return value")

[commit-bot: I haz the power, 2016-08-10 05:02:38]

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-08-10 06:30:46]

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

[commit-bot: I haz the power, 2016-08-10 06:30:47]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[anthonyhkf, 2016-08-10 06:56:34]

The CQ bit was checked by anthonyhkf@google.com

[commit-bot: I haz the power, 2016-08-10 06:57:00]

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-08-10 10:34:45]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-08-10 10:36:02]

Description was changed from

==========
[Typed-OM] Add CSSResourceValue

Spec: https://drafts.css-houdini.org/css-typed-om/#resourcevalue-objects

BUG=545318
==========

to

==========
[Typed-OM] Add CSSResourceValue

Spec: https://drafts.css-houdini.org/css-typed-om/#resourcevalue-objects

BUG=545318

Committed: https://crrev.com/f438b039b4f3d4f8875e71e8387ac3ddcbc2fb7e
Cr-Commit-Position: refs/heads/master@{#411020}
==========

[commit-bot: I haz the power, 2016-08-10 10:36:04]

Patchset 7 (id:??) landed as
https://crrev.com/f438b039b4f3d4f8875e71e8387ac3ddcbc2fb7e
Cr-Commit-Position: refs/heads/master@{#411020}