Settings: use hidden for incognito checkbox

Bug: 849811
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib034544b40403e47e1c7c538b70b93e9d3f6ee8c
Reviewed-on: https://chromium-review.googlesource.com/1087783
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565457}

chrome/browser/resources/settings/site_settings/add_site_dialog.html

@@ -14,12 +14,7 @@
   <template>
     <style include="settings-shared">
       #incognito {
-        margin-bottom: 2px;
-        margin-top: 15px;
-      }
-
-      [invisible] {
-        visibility: hidden;
+        padding-bottom: 10px;
       }
     </style>
     <cr-dialog id="dialog" close-text="$i18n{close}">
@@ -31,7 +26,8 @@
             error-message="$i18n{notValidWebAddress}" spellcheck="false"
             autofocus></paper-input>
         <cr-checkbox id="incognito"
-            invisible$="[[!showIncognitoSessionOnly_]]">
+            hidden$="[[!showIncognitoSessionOnly_(hasIncognito,
+                contentSetting)]]">
           $i18n{incognitoSiteOnly}
         </cr-checkbox>
       </div>

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

@@ -26,7 +26,10 @@ Polymer({
     contentSetting: String,
 
     /** @private */
-    showIncognitoSessionOnly_: Boolean,
+    hasIncognito: {
+      type: Boolean,
+      observer: 'hasIncognitoChanged_',
+    },
 
     /**
      * The site to add an exception for.
@@ -39,17 +42,8 @@ Polymer({
   attached: function() {
     assert(this.category);
     assert(this.contentSetting);
-  },
+    assert(typeof this.hasIncognito != 'undefined');
 
-  /** Open the dialog. */
-  open: function() {
-    this.addWebUIListener('onIncognitoStatusChanged', hasIncognito => {
-      this.$.incognito.checked = false;
-      this.showIncognitoSessionOnly_ = hasIncognito &&
-          !loadTimeData.getBoolean('isGuest') &&
-          this.contentSetting != settings.ContentSetting.SESSION_ONLY;
-    });
-    this.browserProxy.updateIncognitoStatus();
     this.$.dialog.showModal();
   },
 
@@ -90,4 +84,16 @@ Polymer({
         this.$.incognito.checked);
     this.$.dialog.close();
   },
+
+  /** @private */
+  showIncognitoSessionOnly_: function() {
+    return this.hasIncognito && !loadTimeData.getBoolean('isGuest') &&
+        this.contentSetting != settings.ContentSetting.SESSION_ONLY;
+  },
+
+  /** @private */
+  hasIncognitoChanged_: function() {
+    if (!this.hasIncognito)
+      this.$.incognito.checked = false;
+  },
 });

chrome/browser/resources/settings/site_settings/site_list.html

@@ -115,6 +115,12 @@
           on-close="onEditExceptionDialogClosed_">
       </settings-edit-exception-dialog>
     </template>
+    <template is="dom-if" if="[[showAddSiteDialog_]]" restamp>
+      <add-site-dialog has-incognito="[[hasIncognito_]]" category="[[category]]"
+          content-setting="[[categorySubtype]]"
+          on-close="onAddSiteDialogClosed_">
+      </add-site-dialog>
+    </template>
   </template>
   <script src="site_list.js"></script>
 </dom-module>

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

@@ -65,6 +65,12 @@ Polymer({
       value: settings.INVALID_CATEGORY_SUBTYPE,
     },
 
+    /** @private */
+    hasIncognito_: Boolean,
+
+    /** @private */
+    showAddSiteDialog_: Boolean,
+
     /**
      * Whether to show the Allow action in the action menu.
      * @private
@@ -115,6 +121,7 @@ Polymer({
         this.siteWithinCategoryChanged_.bind(this));
     this.addWebUIListener(
         'onIncognitoStatusChanged', this.onIncognitoStatusChanged_.bind(this));
+    this.browserProxy.updateIncognitoStatus();
   },
 
   /**
@@ -134,7 +141,9 @@ Polymer({
    * Another message is sent when the *last* incognito window closes.
    * @private
    */
-  onIncognitoStatusChanged_: function() {
+  onIncognitoStatusChanged_: function(hasIncognito) {
+    this.hasIncognito_ = hasIncognito;
+
     // The SESSION_ONLY list won't have any incognito exceptions. (Minor
     // optimization, not required).
     if (this.categorySubtype == settings.ContentSetting.SESSION_ONLY)
@@ -202,23 +211,17 @@ Polymer({
 
   /**
    * A handler for the Add Site button.
-   * @param {!Event} e
    * @private
    */
-  onAddSiteTap_: function(e) {
+  onAddSiteTap_: function() {
     assert(!this.readOnlyList);
-    e.preventDefault();
-    const dialog = document.createElement('add-site-dialog');
-    dialog.category = this.category;
-    dialog.contentSetting = this.categorySubtype;
-    this.shadowRoot.appendChild(dialog);
-
-    dialog.open(this.categorySubtype);
+    this.showAddSiteDialog_ = true;
+  },
 
-    dialog.addEventListener('close', () => {
+  /** @private */
+  onAddSiteDialogClosed_: function() {
+    this.showAddSiteDialog_ = false;
     cr.ui.focusWithoutInk(assert(this.$.addSite));
-      dialog.remove();
-    });
   },
 
   /**

chrome/test/data/webui/settings/site_list_tests.js

@@ -991,8 +991,8 @@ suite('AddExceptionDialog', function() {
     dialog = document.createElement('add-site-dialog');
     dialog.category = settings.ContentSettingsTypes.GEOLOCATION;
     dialog.contentSetting = settings.ContentSetting.ALLOW;
+    dialog.hasIncognito = false;
     document.body.appendChild(dialog);
-    dialog.open();
   });
 
   teardown(function() {
@@ -1000,15 +1000,13 @@ suite('AddExceptionDialog', function() {
   });
 
   test('incognito', function() {
-    cr.webUIListenerCallback(
-        'onIncognitoStatusChanged',
-        /*hasIncognito=*/true);
+    dialog.set('hasIncognito', true);
+    Polymer.dom.flush();
     assertFalse(dialog.$.incognito.checked);
     dialog.$.incognito.checked = true;
     // Changing the incognito status will reset the checkbox.
-    cr.webUIListenerCallback(
-        'onIncognitoStatusChanged',
-        /*hasIncognito=*/false);
+    dialog.set('hasIncognito', false);
+    Polymer.dom.flush();
     assertFalse(dialog.$.incognito.checked);
   });