WebUI NTP: most-visited, disable save in tile dialog if URL is not valid

Bug: 1069056
Change-Id: Iab2cbb778fbd97c5533388949dd7eb03014ff5fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568542
Auto-Submit: Esmael Elmoslimany <aee@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#832872}

diff --git a/chrome/browser/resources/new_tab_page/most_visited.html b/chrome/browser/resources/new_tab_page/most_visited.html
index 1a90986..21ebb56 100644
--- a/chrome/browser/resources/new_tab_page/most_visited.html
+++ b/chrome/browser/resources/new_tab_page/most_visited.html

@@ -235,7 +235,8 @@
           value="{{dialogTileTitle_}}" spellcheck="false" autofocus></cr-input>
       <cr-input id="dialogInputUrl" label="$i18n{urlField}"
           value="{{dialogTileUrl_}}" invalid="[[dialogTileUrlInvalid_]]"
-          error-message="$i18n{invalidUrl}" spellcheck="false" type="url">
+          error-message="$i18n{invalidUrl}" spellcheck="false" type="url"
+          on-blur="onDialogTileUrlBlur_">
       </cr-input>
     </div>
     <div slot="button-container">
@@ -243,7 +244,7 @@
         $i18n{linkCancel}
       </cr-button>
       <cr-button class="action-button" on-click="onSave_"
-          disabled$="[[!dialogTileUrl_]]">
+          disabled$="[[dialogSaveDisabled_]]">
         $i18n{linkDone}
       </cr-button>
     </div>

diff --git a/chrome/browser/resources/new_tab_page/most_visited.js b/chrome/browser/resources/new_tab_page/most_visited.js
index a2b766e461..0d30802a 100644
--- a/chrome/browser/resources/new_tab_page/most_visited.js
+++ b/chrome/browser/resources/new_tab_page/most_visited.js

@@ -66,6 +66,25 @@
       r => x >= r.left && x <= r.right && y >= r.top && y <= r.bottom);
 }
 
+
+/**
+ * Returns null if URL is not valid.
+ * @param {string} urlString
+ * @return {URL}
+ * @private
+ */
+function normalizeUrl(urlString) {
+  try {
+    const url = new URL(
+        urlString.includes('://') ? urlString : `https://${urlString}/`);
+    if (['http:', 'https:'].includes(url.protocol)) {
+      return url;
+    }
+  } catch (e) {
+  }
+  return null;
+}
+
 class MostVisitedElement extends PolymerElement {
   static get is() {
     return 'ntp-most-visited';
@@ -126,6 +145,12 @@
       /** @private */
       dialogTitle_: String,
 
+      /** @private */
+      dialogSaveDisabled_: {
+        type: Boolean,
+        computed: 'computeDialogSaveDisabled_(dialogTitle_, dialogTileUrl_)',
+      },
+
       /**
        * Used to hide hover style and cr-icon-button of tiles while the tiles
        * are being reordered.
@@ -341,6 +366,14 @@
   }
 
   /**
+   * @return {boolean}
+   * @private
+   */
+  computeDialogSaveDisabled_() {
+    return !this.dialogTileUrl_ || normalizeUrl(this.dialogTileUrl_) === null;
+  }
+
+  /**
    * If a pointer is over a tile rect that is different from the one being
    * dragged, the dragging tile is moved to the new position. The reordering
    * is done in the DOM and the by the |reorderMostVisitedTile()| call. This is
@@ -558,6 +591,14 @@
   }
 
   /** @private */
+  onDialogTileUrlBlur_() {
+    if (this.dialogTileUrl_.length > 0 &&
+        normalizeUrl(this.dialogTileUrl_) === null) {
+      this.dialogTileUrlInvalid_ = true;
+    }
+  }
+
+  /** @private */
   onDialogTileUrlChange_() {
     this.dialogTileUrlInvalid_ = false;
   }
@@ -642,36 +683,21 @@
 
   /** @private */
   async onSave_() {
-    let newUrl;
-    try {
-      newUrl = new URL(
-          this.dialogTileUrl_.includes('://') ?
-              this.dialogTileUrl_ :
-              `https://${this.dialogTileUrl_}/`);
-      if (!['http:', 'https:'].includes(newUrl.protocol)) {
-        throw new Error();
-      }
-    } catch (e) {
-      this.dialogTileUrlInvalid_ = true;
-      return;
-    }
-
-    this.dialogTileUrlInvalid_ = false;
-
+    const newUrl = {url: normalizeUrl(this.dialogTileUrl_).href};
     this.$.dialog.close();
     let newTitle = this.dialogTileTitle_.trim();
     if (newTitle.length === 0) {
       newTitle = this.dialogTileUrl_;
     }
     if (this.adding_) {
-      const {success} = await this.pageHandler_.addMostVisitedTile(
-          {url: newUrl.href}, newTitle);
+      const {success} =
+          await this.pageHandler_.addMostVisitedTile(newUrl, newTitle);
       this.toast_(success ? 'linkAddedMsg' : 'linkCantCreate', success);
     } else {
       const {url, title} = this.tiles_[this.actionMenuTargetIndex_];
-      if (url.url !== newUrl.href || title !== newTitle) {
+      if (url.url !== newUrl.url || title !== newTitle) {
         const {success} = await this.pageHandler_.updateMostVisitedTile(
-            url, {url: newUrl.href}, newTitle);
+            url, newUrl, newTitle);
         this.toast_(success ? 'linkEditedMsg' : 'linkCantEdit', success);
       }
       this.actionMenuTargetIndex_ = -1;

diff --git a/chrome/test/data/webui/new_tab_page/most_visited_test.js b/chrome/test/data/webui/new_tab_page/most_visited_test.js
index f1886e7..b489bfb 100644
--- a/chrome/test/data/webui/new_tab_page/most_visited_test.js
+++ b/chrome/test/data/webui/new_tab_page/most_visited_test.js

@@ -445,10 +445,12 @@
     });
 
     test('http is a valid scheme', async () => {
+      assertTrue(saveButton.disabled);
       inputUrl.value = 'http://url';
       const addCalled = testProxy.handler.whenCalled('addMostVisitedTile');
       saveButton.click();
       await addCalled;
+      assertFalse(saveButton.disabled);
     });
 
     test('https is a valid scheme', async () => {
@@ -459,16 +461,18 @@
     });
 
     test('chrome is not a valid scheme', () => {
+      assertTrue(saveButton.disabled);
       inputUrl.value = 'chrome://url';
       assertFalse(inputUrl.invalid);
-      saveButton.click();
+      mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur'));
       assertTrue(inputUrl.invalid);
+      assertTrue(saveButton.disabled);
     });
 
     test('invalid cleared when text entered', () => {
       inputUrl.value = '%';
       assertFalse(inputUrl.invalid);
-      saveButton.click();
+      mostVisited.$.dialogInputUrl.dispatchEvent(new Event('blur'));
       assertTrue(inputUrl.invalid);
       inputUrl.value = '';
       assertFalse(inputUrl.invalid);
@@ -533,7 +537,7 @@
       inputUrl.value = 'updated-url';
       assertFalse(mostVisited.$.toast.open);
       saveButton.click();
-      await flushTasks();
+      await testProxy.handler.whenCalled('updateMostVisitedTile');
       assertTrue(mostVisited.$.toast.open);
     });