MD Settings: Internet: Add/fix remaining policy items

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

Index: chrome/browser/resources/settings/internet_page/network_proxy.js
diff --git a/chrome/browser/resources/settings/internet_page/network_proxy.js b/chrome/browser/resources/settings/internet_page/network_proxy.js
index 974de27c38904e66d76531285ca14bfb7952a91b..54d516c20250084e6515018e1ecd906d45d2fedc 100644
--- a/chrome/browser/resources/settings/internet_page/network_proxy.js
+++ b/chrome/browser/resources/settings/internet_page/network_proxy.js
@@ -346,7 +346,7 @@ Polymer({
    * @return {boolean}
    * @private
    */
-  getShowPrefPolicyIndicator_: function() {
+  getShowExtensionPolicyIndicator_: function() {

[Dan Beam, 2016/11/29 05:25:34]

why does this use 2 verbs? "get" AND "show"?  why not
shouldShowExtensionIndicator()?  or just isExtensionControlled?

also, why both "Policy" and "Extension"?  that's really confusing...

[stevenjb, 2016/11/30 22:58:19]

-> shouldShowExtensionIndicator

     let property = this.getProxySettingsTypeProperty_();
     return !!property && this.isExtensionControlled(property);
   },
@@ -389,7 +389,7 @@ Polymer({
    */
   isProxyEditable_: function() {
     let property = this.getProxySettingsTypeProperty_();
-    return !!property && this.isEditable_(property);
+    return !property || this.isEditable_(property);

[Dan Beam, 2016/11/29 05:25:34]

can you test this logic?

[stevenjb, 2016/11/30 22:58:19]

We *should* test all of 'internet'. We currently test *none* of it. The testing
is... daunting. I have been suggesting that adding tests would be a great way
for someone else to become familiar with this rather large amount of code for a
while now...

[Dan Beam, 2016/12/02 00:23:50]

now's a great time to start.

[Dan Beam, 2016/12/02 00:59:49]

let me expound on this: the more code you write, the more daunting this will be.
 i don't think "there's no tests and we wrote a lot of code that's not tested,
and will likely continue to add more untested code" is a good excuse.

anyways, as this will ultimately be your burden to bear, i'll leave you to
decide.

[stevenjb, 2016/12/02 17:36:58]

I am not unaware of any of this. The question has always been, "is it worth
taking a week or two to write some tests for this section that might actually
catch regressions?" So far, my response to myself (and I though we had also
discussed this) has been, "this would be a great way to get someone else up to
speed on this code so that I am not the sole maintainer" (since writing useful
tests will require understanding the model API that the UI is built on). Also,
"we really need to ship this sooner than late,r and tests are more valuable (in
my experience) for long term maintenance than short term stability (which is not
to say that tests don't catch short term bugs, but I find that to be more true
with complex model code, which in this case is pretyt well tested, than with UI
controller code).

Anyway, your suggestion is duly noted.

   },
 
   /**