ONC: Translate EAP from Shill to ONC

chrome/browser/resources/settings/internet_page/network_property_list.js

Index: chrome/browser/resources/settings/internet_page/network_property_list.js
diff --git a/chrome/browser/resources/settings/internet_page/network_property_list.js b/chrome/browser/resources/settings/internet_page/network_property_list.js
index d64e6225395d45b7563c9c923de695370ae7b790..4046fc43c7b352c89a9706113871e1835ceb02a1 100644
--- a/chrome/browser/resources/settings/internet_page/network_property_list.js
+++ b/chrome/browser/resources/settings/internet_page/network_property_list.js
@@ -109,7 +109,7 @@ Polymer({
     return function(key) {
       if (this.editFieldTypes.hasOwnProperty(key))
         return true;
-      var value = this.get(key, this.propertyDict);
+      var value = this.getPropertyValue_(key, this.prefix, this.propertyDict);

[tbarzic, 2017/05/01 18:05:56]

the rest of the private methods pass in prefix, propertyDict and editFieldTypes
as function parameters. Should that be the case here, too?

(I'd at least update computeFiter_ invocations in network_property_list.html to
match the method signature here).

[stevenjb, 2017/05/01 20:12:04]

I'm not sure what the relevance of "the rest of the private methods" is, but you
are correct that computeFilter should be passed prefix from the HTML, so I fixed
that.

[tbarzic, 2017/05/01 21:04:48]

I mean, the relevance is consistency, e.g. isEditable_ takes arguments passed
from HTML, and uses those to get result, while computeFilter_ uses the network
list element properties directly (though, arguably, this is existing problem,
not one added with this cl).

If I'm reading the code correctly, this implementation ignores arguments passed
from HTML template.
So, I'd either change this to
computeFilter_ = function(prefix, propertyDict, editFieldTypes) {
  return function(key) {
     // same logic, but using [prefix, propertyDict, editFieldTypes] instead of
[this.prefix, this.propertyDict, this.editFieldTypes]
  }.bind(this);
}
or change invocation from network_property_list.html to
  filter="[[computeFilter_()]]"


(personally, I prefer the latter, but the former is more consistent with the
rest of this file).

[stevenjb, 2017/05/01 21:54:26]

Oh, yeah, that's complicated.

*Generally* we prefer to use members explicitly in functions, i.e. this.prefix,
and provide them as parameters in the HTML for generating dependencies.

NOTE: Providing the parameters in the HTML is essential to create dependencies
(i.e. if |prefix| changes, be sure to call computeFilter_), even if we ignore
them in the method.

However, when -inside- a template, using members doesn't work reliably (the
member may still have the old value), so it is safer to pass members explicitly.

Since computeFilter_ is the only function in this pseudo-class that uses members
instead of parameters, I can see how it would be more clear if we also used
explicit parameters here, so I will do that.

(Sadly we do not have any type checking between the HTML and the JS, and there
are subtle problems with both approaches.)

       return value !== undefined && value !== '';
     }.bind(this);
   },