Site Settings Desktop: Fix bug with deleting manually added exceptions.

chrome/browser/resources/settings/site_settings/site_details_permission.js

Index: chrome/browser/resources/settings/site_settings/site_details_permission.js
diff --git a/chrome/browser/resources/settings/site_settings/site_details_permission.js b/chrome/browser/resources/settings/site_settings/site_details_permission.js
index ee1ec85afd590d7a73f7a7c34cd9b2c5fafd0cd9..ce43171516f798b4c2f79c2579da8d5f8387edb1 100644
--- a/chrome/browser/resources/settings/site_settings/site_details_permission.js
+++ b/chrome/browser/resources/settings/site_settings/site_details_permission.js
@@ -37,8 +37,8 @@ Polymer({
    * @private
    */
   sameOrigin_: function(left, right) {
-    return this.removePatternWildcard_(left) ==
-        this.removePatternWildcard_(right);
+    return this.removePatternWildcard(left) ==
+        this.removePatternWildcard(right);
   },
 
   /**
@@ -52,9 +52,11 @@ Polymer({
     this.browserProxy.getExceptionList(this.category).then(
         function(exceptionList) {
       for (var i = 0; i < exceptionList.length; ++i) {
-        if (this.sameOrigin_(exceptionList[i].origin, site.origin)) {
+        if (this.sameOrigin_(exceptionList[i].origin, site.origin) &&
+            exceptionList[i].embeddingOrigin == site.embeddingOrigin) {

[dschuyler, 2016/08/24 22:27:38]

Optional:
function calls are slow, so this may be faster
if the tests on either side of the '&&' are reversed.
So that the function call doesn't happen if the first
test fails.

[Finnur, 2016/08/25 10:29:04]

Done.

           this.$.permission.selected = exceptionList[i].setting;
           this.$.details.hidden = false;
+          break;
         }
       }
     }.bind(this));
@@ -63,19 +65,20 @@ Polymer({
   /**
    * Called when a site within a category has been changed.
    * @param {number} category The category that changed.
-   * @param {string} site The site that changed.
+   * @param {string} origin The origin of the site that changed.
+   * @param {string} embeddingOrigin The embedding origin of the site that
+   *     changed.
    * @private
    */
-  sitePermissionChanged_: function(category, site) {
-    if (category == this.category && (site == '' || site == this.site.origin)) {
-      // TODO(finnur): Send down the full SiteException, not just a string.
-      this.siteChanged_({
-        origin: site,
-        originForDisplay: '',
-        embeddingOrigin: '',
-        setting: '',
-        source: '',
-      });
+  sitePermissionChanged_: function(category, origin, embeddingOrigin) {
+    if (this.site == undefined)

[dschuyler, 2016/08/24 22:27:38]

Optional: I'd go for === when comparing to undefined.

(if you're really going for 'similar to undefined' 
rather than 'exactly undefined' then maybe go with 
(!this.site) instead).

[Finnur, 2016/08/25 10:29:04]

Done.

+      return;
+    if (category != this.category)
+      return;
+
+    if (origin == '' || (origin == this.site.origin &&
+                         embeddingOrigin == this.site.embeddingOrigin)) {
+      this.siteChanged_(this.site);
     }
   },
 
@@ -83,7 +86,8 @@ Polymer({
    * Resets the category permission for this origin.
    */
   resetPermission: function() {
-    this.resetCategoryPermissionForOrigin(this.site.origin, '', this.category);
+    this.resetCategoryPermissionForOrigin(
+        this.site.origin, this.site.embeddingOrigin, this.category);
     this.$.details.hidden = true;
   },
 
@@ -94,6 +98,6 @@ Polymer({
   onPermissionMenuIronActivate_: function(event) {
     var value = event.detail.item.dataset.permissionValue;
     this.setCategoryPermissionForOrigin(
-        this.site.origin, '', this.category, value);
+        this.site.origin, this.site.embeddingOrigin, this.category, value);
   },
 });