[Chrome Settings UI] Show overruled User Exceptions as strike-through.

chrome/browser/resources/options/content_settings_exceptions_area.js

Index: chrome/browser/resources/options/content_settings_exceptions_area.js
diff --git a/chrome/browser/resources/options/content_settings_exceptions_area.js b/chrome/browser/resources/options/content_settings_exceptions_area.js
index 3ad7ac62a552f0e1079168e4d2c97cf12e4e61c1..05a940cb8aa559c44e6f93ed06211ed5d3d74963 100644
--- a/chrome/browser/resources/options/content_settings_exceptions_area.js
+++ b/chrome/browser/resources/options/content_settings_exceptions_area.js
@@ -345,6 +345,27 @@ cr.define('options.contentSettings', function() {
         settingOption.selected = true;
     },
 
+    /**
+     * Updates UI to indicate that the exception was overruled by a source.
+     *
+     * @param {string} overruledBy The source that overrules the exception.
+     */
+    setOverruledBy: function(overruledBy) {
+      this.classList.toggle('overruled', !!overruledBy);
+      var textElt = this.querySelector('.exception-pattern .static-text');
+      textElt.setAttribute('title',
+          loadTimeData.getString('exceptionDisabledByPolicy'));
+
+      var indicator = new ControlledSettingIndicator();
+      indicator.setAttribute('content-exception', this.contentType);
+      // Create a synthetic pref change event decorated as
+      // CoreOptionsHandler::CreateValueForPref() does.
+      var event = new Event(this.contentType);
+      event.value = { controlledBy: overruledBy };
+      indicator.handlePrefChange(event);
+      this.appendChild(indicator);

[Bernhard Bauer, 2016/05/18 09:55:13]

What happens if this is called multiple times (e.g. if content settings change)?
Would we just append indicators?

[huangs, 2016/05/18 13:52:49]

If content settings change then the entire list gets regenerated.  However,
relying on call patterns seem kinda fragile.  Please confirm you want
unification code, then I'll add it (extra unused complexity to be defensive).

+    },
+
     /** @override */
     get currentInputIsValid() {
       return this.inputValidityKnown && this.inputIsValid;
@@ -520,6 +541,20 @@ cr.define('options.contentSettings', function() {
     },
 
     /**
+     * Updates UI to indicate that user exceptions were overruled by a source.
+     *
+     * @param {string} overruledBy The source that overrules user exceptions.
+     */
+    setOverruledBy: function(overruledBy) {
+      for (var index = 0; index < this.dataModel.length; ++index) {
+        var item = this.getListItemByIndex(index);
+        if (item.dataItem.source == 'preference') {

[Bernhard Bauer, 2016/05/18 09:55:13]

Doesn't this mean that any preference rule is marked as overruled by any
higher-level rule, even if the higher-level rule doesn't apply to the same
origins? E.g., if an extension sets a content setting for google.com, a
preferece rule for yahoo.com would show up as overruled?

[huangs, 2016/05/18 13:52:49]

The "overruled" concept only applies to Policy Defaults and Extension Defaults. 
We don't count exception overrides (inlcuding wildcards).

[Bernhard Bauer, 2016/05/19 14:20:40]

I don't really want to get into a product discussion, but... why don't we?
Historically, we tried not to treat default settings vastly different from
exceptions (default settings are just rules with wildcard patterns).

[huangs, 2016/05/20 15:16:46]

I think treating default as "super wildcard exception" would belong in the
"model" layer.  However, we separate these in the UI, so having a separate
treatment would belong in the "view" and "controller" layer, so maybe there's no
conflict fundamentally?

[Bernhard Bauer, 2016/05/23 13:34:11]

I'm not sure what you are trying to say. This UI will not in all cases where a
rule does not have any effect show it as overruled, which I would consider a
bug. It's true that the UI for a default setting looks different from the UI for
an exception, but
a) that is only the case for user-defined content settings, and this is for
policy- or extension-defined content settings, and
b) the question of whether a rule is effective or not (which is what we want to
show here) is independent of the UI treatment.

I mean, if you want to land this, I won't stop you, but like I said above, to me
this code is not free of bugs, and I would like to see them addressed at least
in the future.

[huangs, 2016/05/24 05:02:00]

By "bug" do you mean:

(1) Code is not doing what is specified, or
(2) The specification is wrong?

My take: The specification is "A User Exception is 'overruled' if it becomes
ineffective due to Policy Default or Extension Default settings.  For
'overruled' user exception, change UI to draw line-through.".

Are you saying that the spec should be: "A User Exception is 'overruled' if
anything, i.e., {Policy, Extension} {Default, Exceptions} make it ineffective. 
For 'overruled' user exception, change UI to..."

If so, then we are disagreeing on (2), and so I'd think "bug" is too strong;
perhaps "incomplete fix"?

If you mean something else, then sorry for misunderstanding!  I'd like to sort
this out before proceeding.

[Bernhard Bauer, 2016/05/24 16:54:34]

Yes, that's what I mean.
What's the difference? I mean, I agree that this change is better than nothing
(which is why I'm not objecting to it in principle), but the part not fixed by
an incomplete fix would be a bug, no?

+          item.setOverruledBy(overruledBy);
+        }
+      }
+    },
+
+    /**
      * Sets the exceptions in the js model.
      *
      * @param {Array<options.Exception>} entries A list of dictionaries of