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

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * 'site-details-permission' handles showing the state of one permission, such
  * as Geolocation, for a given origin.
  */
 Polymer({
@@ skipping 19 matching lines @@
 
   /**
    * Returns true if the origins match, e.g. http://google.com and
    * http://[*.]google.com.
    * @param {string} left The first origin to compare.
    * @param {string} right The second origin to compare.
    * @return {boolean} True if the origins are the same.
    * @private
    */
   sameOrigin_: function(left, right) {
-    return this.removePatternWildcard_(left) ==
-        this.removePatternWildcard_(right);
+    return this.removePatternWildcard(left) ==
+        this.removePatternWildcard(right);
   },
 
   /**
    * Sets the site to display.
    * @param {!SiteException} site The site to display.
    * @private
    */
   siteChanged_: function(site) {
     this.$.details.hidden = true;
 
     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));
   },
 
   /**
    * 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);
     }
   },
 
   /**
    * 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;
   },
 
   /**
    * Handles the category permission changing for this origin.
    * @param {!{detail: !{item: !{dataset: !{permissionValue: string}}}}} event
    */
   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);
   },
 });