Fixes wrong inheritance of PrefSelect.

Fixes wrong inheritance of PrefSelect.

PrefSelect must inherit from HTMLSelectElement, otherwise it will be
broken when the DOM attributes are moved to prototype chains.

Note that HTMLSelectElement does NOT inherit from HTMLInputElement, so
it's simply wrong that PrefSelect inherits from HTMLInputElement.

BUG=43394
Committed: https://crrev.com/ac0bacffa7b4f25a09abaa082a32abb599d6e318
Cr-Commit-Position: refs/heads/master@{#327023}

[Yuki, 2015-04-20 10:42:45]

yukishiino@chromium.org changed reviewers:
+ dbeam@chromium.org

[Yuki, 2015-04-20 10:42:45]

Could you review this CL?

[Dan Beam, 2015-04-20 15:06:27]

I am not sure I'm the best reviewer for this.

+jhawkins, estade, arv for their opinions

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/pref_ui.js (right):

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/pref_ui.js:393: __proto__:
HTMLSelectElement.prototype,
keep in mind that settings is being rewritten actively

i think it's better to do one of two things:
1) make an abstract class that is concretely derived into
Pref{Input,Select}Element
2) make a concrete base class that has-an element (rather than is-an element)

both of these sound mildly messy and slightly error prone (but at least we have
typechecking now)

[James Hawkins, 2015-04-20 15:20:09]

On 2015/04/20 15:06:27, Dan Beam wrote:
> I am not sure I'm the best reviewer for this.
> 
> +jhawkins, estade, arv for their opinions
> 

I defer to arv on this one, as he's been most involved in the discussions
leading up to the need for this CL (on 43394).

[Yuki, 2015-04-21 04:37:13]

On 2015/04/20 15:20:09, James Hawkins wrote:
> On 2015/04/20 15:06:27, Dan Beam wrote:
> > I am not sure I'm the best reviewer for this.
> > 
> > +jhawkins, estade, arv for their opinions
> > 
> 
> I defer to arv on this one, as he's been most involved in the discussions
> leading up to the need for this CL (on 43394).

Let me clarify my goal.  My purpose on this CL is fix Blink's DOM attribute
implementation (i.e. move DOM attributes to prototype chains in Blink).

I'd like to defer it to you guys to bring the best design to cr.ui.
My only purpose here is simply fix the broken inheritance.  Please refactor
cr.ui later for yourselves if needed.

[Dan Beam, 2015-04-21 16:22:14]

dbeam@chromium.org changed reviewers:
+ arv@chromium.org
- dbeam@chromium.org

[Dan Beam, 2015-04-21 16:22:38]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2015-04-21 16:22:39]

-dbeam, +arv

[Dan Beam, 2015-04-21 23:00:00]

dbeam@chromium.org changed reviewers:
- dbeam@chromium.org

[Yuki, 2015-04-23 04:54:17]

Friendly ping?

arv@, could you review this CL?

[arv (Not doing code reviews), 2015-04-24 14:22:46]

LGTM

Since Dan said they are rewriting prefs I don't think you need to refactor prefs
to make this better at this point.

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/pref_ui.js (right):

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/pref_ui.js:393: __proto__:
HTMLSelectElement.prototype,
On 2015/04/20 15:06:26, Dan Beam wrote:
> keep in mind that settings is being rewritten actively
> 
> i think it's better to do one of two things:
> 1) make an abstract class that is concretely derived into
> Pref{Input,Select}Element
> 2) make a concrete base class that has-an element (rather than is-an element)
> 
> both of these sound mildly messy and slightly error prone (but at least we
have
> typechecking now)

With ES6 you can do traits.

function PrefTrait(superClass) {
  return class extends superClass {
    handleChange(e) { ... }
  };
}

class PrefSelect extends PrefTrait(HTMLSelectElement) {
  ...
}

class PrefInputElement extends PrefTrait(HTMLInputElement) {
  ...
}

Of course you can desugar that to ES5 if you want.

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/pref_ui.js:473: * Custom change handler that is
invoked first when the user makes changes
Are these comments just copied from PrefInputElement? If so remove them and add
something like "see PrefInputElement".

[Dan Beam, 2015-04-24 21:48:29]

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/pref_ui.js (right):

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/pref_ui.js:473: * Custom change handler that is
invoked first when the user makes changes
On 2015/04/24 14:22:45, arv wrote:
> Are these comments just copied from PrefInputElement? If so remove them and
add
> something like "see PrefInputElement".

or [just] @override

[Yuki, 2015-04-27 09:22:38]

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
File chrome/browser/resources/options/pref_ui.js (right):

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/pref_ui.js:393: __proto__:
HTMLSelectElement.prototype,
On 2015/04/24 14:22:45, arv wrote:
> On 2015/04/20 15:06:26, Dan Beam wrote:
> > keep in mind that settings is being rewritten actively
> > 
> > i think it's better to do one of two things:
> > 1) make an abstract class that is concretely derived into
> > Pref{Input,Select}Element
> > 2) make a concrete base class that has-an element (rather than is-an
element)
> > 
> > both of these sound mildly messy and slightly error prone (but at least we
> have
> > typechecking now)
> 
> With ES6 you can do traits.
> 
> function PrefTrait(superClass) {
>   return class extends superClass {
>     handleChange(e) { ... }
>   };
> }
> 
> class PrefSelect extends PrefTrait(HTMLSelectElement) {
>   ...
> }
> 
> class PrefInputElement extends PrefTrait(HTMLInputElement) {
>   ...
> }
> 
> Of course you can desugar that to ES5 if you want.

Looks cool to me.  I defer this re-design task to the maintainers (code owners).

https://codereview.chromium.org/1061263004/diff/1/chrome/browser/resources/op...
chrome/browser/resources/options/pref_ui.js:473: * Custom change handler that is
invoked first when the user makes changes
On 2015/04/24 21:48:29, Dan Beam wrote:
> On 2015/04/24 14:22:45, arv wrote:
> > Are these comments just copied from PrefInputElement? If so remove them and
> add
> > something like "see PrefInputElement".
> 
> or [just] @override

Done.

[Yuki, 2015-04-27 10:26:32]

The CQ bit was checked by yukishiino@chromium.org

[Yuki, 2015-04-27 10:26:33]

The patchset sent to the CQ was uploaded after l-g-t-m from arv@chromium.org
Link to the patchset: https://codereview.chromium.org/1061263004/#ps20001
(title: "Addressed review comments.")

[commit-bot: I haz the power, 2015-04-27 10:26:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1061263004/20001

[commit-bot: I haz the power, 2015-04-27 10:30:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-27 10:31:12]

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