Settings: close dialog or validate inputs when search engine template URL's change

Bug: 781703
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If6fda4644b20764c0561d03a826c97e561f80337
Reviewed-on: https://chromium-review.googlesource.com/1024967
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553757}

chrome/browser/resources/settings/search_engines_page/compiled_resources2.gyp

@@ -8,6 +8,7 @@
       'dependencies': [
         '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:cr',
         '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:load_time_data',
+        '<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:web_ui_listener_behavior',
         'search_engines_browser_proxy',
       ],
       'includes': ['../../../../../third_party/closure_compiler/compile_js2.gypi'],

chrome/browser/resources/settings/search_engines_page/search_engine_dialog.html

 <link rel="import" href="chrome://resources/html/polymer.html">
 
 <link rel="import" href="chrome://resources/cr_elements/cr_dialog/cr_dialog.html">
+<link rel="import" href="chrome://resources/html/web_ui_listener_behavior.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-button/paper-button.html">
 <link rel="import" href="chrome://resources/polymer/v1_0/paper-input/paper-input.html">
 <link rel="import" href="../i18n_setup.html">

chrome/browser/resources/settings/search_engines_page/search_engine_dialog.js

@@ -9,6 +9,8 @@
 Polymer({
   is: 'settings-search-engine-dialog',
 
+  behaviors: [WebUIListenerBehavior],
+
   properties: {
     /**
      * The search engine to be edited. If not populated a new search engine
@@ -69,6 +71,9 @@ Polymer({
     this.addEventListener('cancel', () => {
       this.browserProxy_.searchEngineEditCancelled();
     });
+
+    this.addWebUIListener(
+        'search-engines-changed', this.enginesChanged_.bind(this));
   },
 
   /** @override */
@@ -79,6 +84,25 @@ Polymer({
     this.$.dialog.showModal();
   },
 
+  /**
+   * @param {!SearchEnginesInfo} searchEnginesInfo
+   * @private
+   */
+  enginesChanged_: function(searchEnginesInfo) {
+    if (this.model) {
+      const engineWasRemoved = ['defaults', 'others', 'extensions'].every(
+          engineType =>
+              searchEnginesInfo[engineType].every(e => e.id != this.model.id));
+      if (engineWasRemoved) {
+        this.cancel_();
+        return;
+      }
+    }
+
+    [this.$.searchEngine, this.$.keyword, this.$.queryUrl].forEach(
+        element => this.validateElement_(element));
+  },
+
   /** @private */
   cancel_: function() {
     this.$.dialog.cancel();
@@ -92,12 +116,10 @@ Polymer({
   },
 
   /**
-   * @param {!Event} event
+   * @param {!Element} inputElement
    * @private
    */
-  validate_: function(event) {
-    const inputElement = Polymer.dom(event).localTarget;
-
+  validateElement_: function(inputElement) {
     // If element is empty, disable the action button, but don't show the red
     // invalid message.
     if (inputElement.value == '') {
@@ -114,6 +136,15 @@ Polymer({
         });
   },
 
+  /**
+   * @param {!Event} event
+   * @private
+   */
+  validate_: function(event) {
+    const inputElement = /** @type {!Element} */ (event.target);
+    this.validateElement_(inputElement);
+  },
+
   /** @private */
   updateActionButtonState_: function() {
     const allValid = [

chrome/browser/resources/settings/search_engines_page/search_engines_browser_proxy.js

@@ -18,6 +18,7 @@
  *                         canBeDisabled: boolean,
  *                         icon: string}|undefined),
  *            iconURL: (string|undefined),
+ *            id: number,
  *            isOmniboxExtension: boolean,
  *            keyword: string,
  *            modelIndex: number,

chrome/browser/ui/webui/settings/search_engines_handler.cc

@@ -188,6 +188,7 @@ SearchEnginesHandler::CreateDictionaryForEngine(int index, bool is_default) {
   // in @typedef for SearchEngine. Please update it whenever you add or remove
   // any keys here.
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+  dict->SetInteger("id", template_url->id());
   dict->SetString("name", template_url->short_name());
   dict->SetString("displayName",
                   table_model->GetText(

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

@@ -4,6 +4,7 @@
 
 cr.define('settings_search_engines_page', function() {
   /**
+   * @param {number} id
    * @param {string} name
    * @param {boolean} canBeDefault
    * @param {boolean} canBeEdited
@@ -11,19 +12,20 @@ cr.define('settings_search_engines_page', function() {
    * @return {!SearchEngine}
    */
   function createSampleSearchEngine(
-      name, canBeDefault, canBeEdited, canBeRemoved) {
+      id, name, canBeDefault, canBeEdited, canBeRemoved) {
     return {
       canBeDefault: canBeDefault,
       canBeEdited: canBeEdited,
       canBeRemoved: canBeRemoved,
       default: false,
-      displayName: name + " displayName",
-      iconURL: "http://www.google.com/favicon.ico",
+      displayName: name + ' displayName',
+      iconURL: 'http://www.google.com/favicon.ico',
+      id: id,
       isOmniboxExtension: false,
       keyword: name,
       modelIndex: 0,
       name: name,
-      url: "https://" + name + ".com/search?p=%s",
+      url: 'https://' + name + '.com/search?p=%s',
       urlLocked: false,
     };
   }
@@ -35,17 +37,18 @@ cr.define('settings_search_engines_page', function() {
       canBeEdited: false,
       canBeRemoved: false,
       default: false,
-      displayName: "Omnibox extension displayName",
+      displayName: 'Omnibox extension displayName',
       extension: {
-        icon: "chrome://extension-icon/some-extension-icon",
-        id: "dummyextensionid",
-        name: "Omnibox extension"
+        icon: 'chrome://extension-icon/some-extension-icon',
+        id: 'dummyextensionid',
+        name: 'Omnibox extension'
       },
+      id: 0,
       isOmniboxExtension: true,
-      keyword: "oe",
+      keyword: 'oe',
       modelIndex: 6,
-      name: "Omnibox extension",
-      url: "chrome-extension://dummyextensionid/?q=%s",
+      name: 'Omnibox extension',
+      url: 'chrome-extension://dummyextensionid/?q=%s',
       urlLocked: false
     };
   }
@@ -134,6 +137,29 @@ cr.define('settings_search_engines_page', function() {
               return browserProxy.whenCalled('searchEngineEditCompleted');
             });
       });
+
+      test('DialogCloseWhenEnginesChangedModelEngineNotFound', function() {
+        dialog.set(
+            'model', createSampleSearchEngine(0, 'G', false, false, false));
+        cr.webUIListenerCallback('search-engines-changed', {
+          defaults: [],
+          others: [createSampleSearchEngine(1, 'H', false, false, false)],
+          extensions: [],
+        });
+        return browserProxy.whenCalled('searchEngineEditCancelled');
+      });
+
+      test('DialogValidateInputsWhenEnginesChanged', function() {
+        dialog.set(
+            'model', createSampleSearchEngine(0, 'G', false, false, false));
+        dialog.set('keyword_', 'G');
+        cr.webUIListenerCallback('search-engines-changed', {
+          defaults: [],
+          others: [createSampleSearchEngine(0, 'G', false, false, false)],
+          extensions: [],
+        });
+        return browserProxy.whenCalled('validateSearchEngineInput');
+      });
     });
   }
 
@@ -146,7 +172,7 @@ cr.define('settings_search_engines_page', function() {
       let browserProxy = null;
 
       /** @type {!SearchEngine} */
-      const searchEngine = createSampleSearchEngine('G', true, true, true);
+      const searchEngine = createSampleSearchEngine(0, 'G', true, true, true);
 
       setup(function() {
         browserProxy = new settings_search.TestSearchEnginesBrowserProxy();
@@ -244,25 +270,25 @@ cr.define('settings_search_engines_page', function() {
 
       test('Remove_Disabled', function() {
         testButtonDisabled(
-            createSampleSearchEngine('G', true, true, false), 'delete');
+            createSampleSearchEngine(0, 'G', true, true, false), 'delete');
       });
 
       test('MakeDefault_Disabled', function() {
         testButtonDisabled(
-            createSampleSearchEngine('G', false, true, true), 'makeDefault');
+            createSampleSearchEngine(0, 'G', false, true, true), 'makeDefault');
       });
 
       test('Edit_Disabled', function() {
         testButtonDisabled(
-            createSampleSearchEngine('G', true, false, true), 'edit');
+            createSampleSearchEngine(0, 'G', true, false, true), 'edit');
       });
 
       test('All_Disabled', function() {
-        entry.engine = createSampleSearchEngine('G', true, false, false);
+        entry.engine = createSampleSearchEngine(0, 'G', true, false, false);
         Polymer.dom.flush();
         assertTrue(entry.hasAttribute('show-dots_'));
 
-        entry.engine = createSampleSearchEngine('G', false, false, false);
+        entry.engine = createSampleSearchEngine(1, 'G', false, false, false);
         Polymer.dom.flush();
         assertFalse(entry.hasAttribute('show-dots_'));
       });
@@ -279,10 +305,10 @@ cr.define('settings_search_engines_page', function() {
       /** @type {!SearchEnginesInfo} */
       const searchEnginesInfo = {
         defaults: [createSampleSearchEngine(
-            'search_engine_G', false, false, false)],
+            0, 'search_engine_G', false, false, false)],
         others: [
-          createSampleSearchEngine('search_engine_B', false, false, false),
-          createSampleSearchEngine('search_engine_A', false, false, false),
+          createSampleSearchEngine(1, 'search_engine_B', false, false, false),
+          createSampleSearchEngine(2, 'search_engine_A', false, false, false),
         ],
         extensions: [createSampleOmniboxExtension()],
       };
@@ -356,7 +382,7 @@ cr.define('settings_search_engines_page', function() {
 
         cr.webUIListenerCallback('search-engines-changed', {
           defaults: [],
-          others: [createSampleSearchEngine('G', false, false, false)],
+          others: [createSampleSearchEngine(0, 'G', false, false, false)],
           extensions: [],
         });
         assertTrue(message.hasAttribute('hidden'));