Fix in compiler pass: cr.defineProperty() with 2 arguments means opt_kind == cr.PropertyKind.JS

Fix in compiler pass: cr.defineProperty() with 2 arguments means opt_kind == cr.PropertyKind.JS

BUG=393873
R=tbreisacher@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289902

[Tyler Breisacher (Chromium), 2014-08-14 22:00:28]

lgtm

[Dan Beam, 2014-08-14 22:29:20]

lgtm but i'm not sure what good cr.define(a, 'b') is...

[Tyler Breisacher (Chromium), 2014-08-14 22:30:18]

On 2014/08/14 22:29:20, Dan Beam wrote:
> lgtm but i'm not sure what good cr.define(a, 'b') is...

This is only about cr.defineProperty, not cr.define, if I'm not mistaken.

[Dan Beam, 2014-08-14 22:32:07]

On 2014/08/14 22:30:18, Tyler Breisacher (Chromium) wrote:
> On 2014/08/14 22:29:20, Dan Beam wrote:
> > lgtm but i'm not sure what good cr.define(a, 'b') is...
> 
> This is only about cr.defineProperty, not cr.define, if I'm not mistaken.

that's what i meant.  same question: what's the point of cr.defineProperty(a,
'b')?

[Vitaly Pavlenko, 2014-08-14 22:32:33]

On 2014/08/14 22:30:18, Tyler Breisacher (Chromium) wrote:
> On 2014/08/14 22:29:20, Dan Beam wrote:
> > lgtm but i'm not sure what good cr.define(a, 'b') is...
> 
> This is only about cr.defineProperty, not cr.define, if I'm not mistaken.

Yes, you're right. And it's used in some places:
https://code.google.com/p/chromium/codesearch#search/&q=cr.defineProperty%5C(...

[Tyler Breisacher (Chromium), 2014-08-14 22:33:28]

On 2014/08/14 22:32:33, Vitaly Pavlenko wrote:
> On 2014/08/14 22:30:18, Tyler Breisacher (Chromium) wrote:
> > On 2014/08/14 22:29:20, Dan Beam wrote:
> > > lgtm but i'm not sure what good cr.define(a, 'b') is...
> > 
> > This is only about cr.defineProperty, not cr.define, if I'm not mistaken.
> 
> Yes, you're right. And it's used in some places:
>
https://code.google.com/p/chromium/codesearch#search/&q=cr.defineProperty%5C(...

The point is that it tells the compiler that property is going to be added, I
guess? :)

But you're right, it's probably a no-op at runtime.

[Vitaly Pavlenko, 2014-08-14 22:34:40]

On 2014/08/14 22:32:07, Dan Beam wrote:
> On 2014/08/14 22:30:18, Tyler Breisacher (Chromium) wrote:
> > On 2014/08/14 22:29:20, Dan Beam wrote:
> > > lgtm but i'm not sure what good cr.define(a, 'b') is...
> > 
> > This is only about cr.defineProperty, not cr.define, if I'm not mistaken.
> 
> that's what i meant.  same question: what's the point of cr.defineProperty(a,
> 'b')?

Sorry, I don't get the question.

Btw, I'm still unhappy because with current solution we lose typechecking for
non-strings and non-booleans:
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...
https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources...

[Dan Beam, 2014-08-14 22:34:42]

On 2014/08/14 22:33:28, Tyler Breisacher (Chromium) wrote:
> On 2014/08/14 22:32:33, Vitaly Pavlenko wrote:
> > On 2014/08/14 22:30:18, Tyler Breisacher (Chromium) wrote:
> > > On 2014/08/14 22:29:20, Dan Beam wrote:
> > > > lgtm but i'm not sure what good cr.define(a, 'b') is...
> > > 
> > > This is only about cr.defineProperty, not cr.define, if I'm not mistaken.
> > 
> > Yes, you're right. And it's used in some places:
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=cr.defineProperty%5C(...
> 
> The point is that it tells the compiler that property is going to be added, I
> guess? :)
> 
> But you're right, it's probably a no-op at runtime.

ah, it dispatches property change events on the first argument (e.g. |a|)

[Dan Beam, 2014-08-14 22:35:25]

On 2014/08/14 22:34:42, Dan Beam wrote:
> On 2014/08/14 22:33:28, Tyler Breisacher (Chromium) wrote:
> > On 2014/08/14 22:32:33, Vitaly Pavlenko wrote:
> > > On 2014/08/14 22:30:18, Tyler Breisacher (Chromium) wrote:
> > > > On 2014/08/14 22:29:20, Dan Beam wrote:
> > > > > lgtm but i'm not sure what good cr.define(a, 'b') is...
> > > > 
> > > > This is only about cr.defineProperty, not cr.define, if I'm not
mistaken.
> > > 
> > > Yes, you're right. And it's used in some places:
> > >
> >
>
https://code.google.com/p/chromium/codesearch#search/&q=cr.defineProperty%5C(...
> > 
> > The point is that it tells the compiler that property is going to be added,
I
> > guess? :)
> > 
> > But you're right, it's probably a no-op at runtime.
> 
> ah, it dispatches property change events on the first argument (e.g. |a|)

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/data/w...

[Vitaly Pavlenko, 2014-08-15 04:20:57]

The CQ bit was checked by vitalyp@chromium.org

[commit-bot: I haz the power, 2014-08-15 04:24:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vitalyp@chromium.org/473853003/1

[commit-bot: I haz the power, 2014-08-15 16:24:33]

Committed patchset #1 (1) as 289902