Added rename session feature.

Bugs: 820546, 820554, 821063
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0022b28d5b3289e9b87f2f06498cbd03212d19c4
Reviewed-on: https://chromium-review.googlesource.com/929754Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Georges Khalil <georgesak@chromium.org>
Reviewed-by: Owen Min <zmin@chromium.org>
Commit-Queue: Martin Rodriguez <rodmartin@google.com>
Cr-Commit-Position: refs/heads/master@{#542818}

chrome/browser/resources/policy_tool.css

@@ -32,6 +32,13 @@ tbody.value-editing-on .value {
   display: block;
 }
 
+#session-rename-error {
+  display: block;
+  color: red;
+  margin: 20px;
+  text-align: center;
+}
+
 #session-list {
   background: white;
 }
@@ -43,3 +50,22 @@ tbody.value-editing-on .value {
 #session-actions {
   float: left;
 }
+
+#session-actions button {
+  display: block;
+  width: 80px;
+}
+
+#rename-dialog {
+  border: 3px black;
+  height: 130px;
+  outline: 2px solid DarkGrey;
+  text-align: center;
+  width: 300px;
+}
+
+#new-session-name-field {
+  display: block;
+  margin: 20px auto;
+  width: 250px
+}

chrome/browser/resources/policy_tool.html

@@ -31,6 +31,8 @@
       <h1>$i18n{title}</h1>
     </header>
     <section class="reload-show-unset-section">
+      <h1 id="session-title">
+      </h1>
       <form id="session-choice">
         <input id="session-name-field" type="text" autocomplete="off"
             placeholder="$i18n{sessionNamePlaceholder}">
@@ -51,8 +53,15 @@
       </span>
     </section>
     <section id="sessions">
+      <dialog id="rename-dialog">
+        <input id="new-session-name-field" type="text">
+        <button id="cancel-rename-button">$i18n{cancelRename}</button>
+        <button id="confirm-rename-button">$i18n{confirmRename}</button>
+        <span id="session-rename-error" hidden></span>
+      </dialog>
       <div id="session-actions">
-        <button id="delete-session-button">-</button>
+        <button id="delete-session-button">$i18n{removeSession}</button>
+        <button id="rename-session-button">$i18n{renameSession}</button>
       </div>
       <select size="4" id="session-list">
       </select>

chrome/browser/resources/policy_tool.js

@@ -21,6 +21,10 @@ policy.Page.setSessionsList = function(sessions) {
   }
 };
 
+policy.Page.setSessionTitle = function(name) {
+  $('session-title').textContent = name;
+};
+
 // Override some methods of policy.Page.
 
 /**
@@ -30,6 +34,23 @@ policy.Page.showInvalidSessionNameError = function() {
   $('invalid-session-name-error').hidden = false;
 };
 
+/**
+ * Shows error message of rename session.
+ */
+policy.Page.showRenameSessionError = function(errorMessage) {
+  $('session-rename-error').hidden = false;
+  $('session-rename-error').textContent = errorMessage;
+};
+
+/**
+ * Close dialog of rename session.
+ */
+policy.Page.closeRenameSessionDialog = function() {
+  $('session-rename-error').textContent = '';
+  $('session-rename-error').hidden = true;
+  $('rename-dialog').close();
+};
+
 /**
  * Disables editing policy values by hiding the main section and shows an
  * error message instead.
@@ -116,6 +137,29 @@ policy.Page.prototype.initialize = function() {
     }
   };
 
+  $('rename-session-button').onclick = () => {
+    $('session-rename-error').hidden = true;
+    var sessionName = $('session-list').value;
+    if (sessionName) {
+      $('rename-dialog').showModal();
+      $('new-session-name-field').value = '';
+      $('new-session-name-field').select();
+    }
+  };
+
+  $('cancel-rename-button').onclick = () => {
+    $('rename-dialog').close();
+  };
+
+  $('confirm-rename-button').onclick = () => {
+    $('session-rename-error').textContent = '';
+    var sessionName = $('session-list').value;
+    var newSessionName = $('new-session-name-field').value;
+    if (sessionName && newSessionName) {
+      chrome.send('renameSession', [sessionName, newSessionName]);
+    }
+  };
+
   // Notify the browser that the page has loaded, causing it to send the
   // list of all known policies and the values from the default session.
   chrome.send('initialized');

chrome/browser/ui/webui/policy_tool_ui.cc

@@ -29,10 +29,14 @@ content::WebUIDataSource* CreatePolicyToolUIHtmlSource() {
   source->AddLocalizedString("hideExpandedValue",
                              IDS_POLICY_HIDE_EXPANDED_VALUE);
   source->AddLocalizedString("loadSession", IDS_POLICY_TOOL_LOAD_SESSION);
+  source->AddLocalizedString("removeSession", IDS_POLICY_TOOL_REMOVE_SESSION);
+  source->AddLocalizedString("renameSession", IDS_POLICY_TOOL_RENAME_SESSION);
   source->AddLocalizedString("sessionNamePlaceholder",
                              IDS_POLICY_TOOL_SESSION_NAME_PLACEHOLDER);
   source->AddLocalizedString("filterPlaceholder",
                              IDS_POLICY_FILTER_PLACEHOLDER);
+  source->AddLocalizedString("cancelRename", IDS_POLICY_TOOL_CANCEL_RENAME);
+  source->AddLocalizedString("confirmRename", IDS_POLICY_TOOL_CONFIRM_RENAME);
   source->AddLocalizedString("edit", IDS_POLICY_TOOL_EDIT);
   source->AddLocalizedString("save", IDS_POLICY_TOOL_SAVE);
   source->AddLocalizedString("errorSavingDisabled",
@@ -42,6 +46,12 @@ content::WebUIDataSource* CreatePolicyToolUIHtmlSource() {
   source->AddLocalizedString("errorFileCorrupted",
                              IDS_POLICY_TOOL_CORRUPTED_FILE);
   source->AddLocalizedString("enableEditing", IDS_POLICY_TOOL_ENABLE_EDITING);
+  source->AddLocalizedString("errorRenameFailed",
+                             IDS_POLICY_TOOL_RENAME_FAILED);
+  source->AddLocalizedString("errorSessionExist",
+                             IDS_POLICY_TOOL_SESSION_EXIST);
+  source->AddLocalizedString("errorSessionNotExist",
+                             IDS_POLICY_TOOL_SESSION_NOT_EXIST);
   source->AddLocalizedString("errorDeleteFailed",
                              IDS_POLICY_TOOL_DELETE_FAILED);
   // Overwrite the title value added by PolicyUIHandler.

chrome/browser/ui/webui/policy_tool_ui_browsertest.cc

@@ -6,6 +6,7 @@
 #include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/path_service.h"
+#include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_restrictions.h"
@@ -40,10 +41,13 @@ class PolicyToolUITest : public InProcessBrowserTest {
 
   void LoadSession(const std::string& session_name);
   void DeleteSession(const std::string& session_name);
+  void RenameSession(const std::string& session_name,
+                     const std::string& new_session_name);
 
   std::unique_ptr<base::DictionaryValue> ExtractPolicyValues(bool need_status);
 
   bool IsInvalidSessionNameErrorMessageDisplayed();
+  bool IsSessionRenameErrorMessageDisplayed();
 
   std::unique_ptr<base::ListValue> ExtractSessionsList();
 
@@ -114,6 +118,18 @@ void PolicyToolUITest::DeleteSession(const std::string& session_name) {
   content::RunAllTasksUntilIdle();
 }
 
+void PolicyToolUITest::RenameSession(const std::string& session_name,
+                                     const std::string& new_session_name) {
+  const std::string javascript =
+      base::StrCat({"$('session-list').value = '", session_name, "';",
+                    "$('rename-session-button').click();",
+                    "$('new-session-name-field').value = '", new_session_name,
+                    "';", "$('confirm-rename-button').click();"});
+  EXPECT_TRUE(content::ExecuteScript(
+      browser()->tab_strip_model()->GetActiveWebContents(), javascript));
+  content::RunAllTasksUntilIdle();
+}
+
 std::unique_ptr<base::DictionaryValue> PolicyToolUITest::ExtractPolicyValues(
     bool need_status) {
   std::string javascript =
@@ -170,6 +186,16 @@ bool PolicyToolUITest::IsInvalidSessionNameErrorMessageDisplayed() {
   return result;
 }
 
+bool PolicyToolUITest::IsSessionRenameErrorMessageDisplayed() {
+  constexpr char kJavascript[] =
+      "domAutomationController.send($('session-rename-error').hidden == false)";
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  bool result = false;
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(contents, kJavascript, &result));
+  return result;
+}
+
 void PolicyToolUITest::CreateMultipleSessionFiles(int count) {
   base::ScopedAllowBlockingForTesting allow_blocking;
   EXPECT_TRUE(base::CreateDirectory(GetSessionsDir()));
@@ -429,3 +455,54 @@ IN_PROC_BROWSER_TEST_F(PolicyToolUITest, DeleteSession) {
   expected.GetList().erase(expected.GetList().begin());
   EXPECT_EQ(expected, *ExtractSessionsList());
 }
+
+IN_PROC_BROWSER_TEST_F(PolicyToolUITest, RenameSession) {
+  CreateMultipleSessionFiles(3);
+  ui_test_utils::NavigateToURL(browser(), GURL("chrome://policy-tool"));
+  EXPECT_EQ("2", ExtractSinglePolicyValue("SessionId"));
+
+  // Check that a non-current session is renamed correctly.
+  RenameSession("0", "4");
+  EXPECT_FALSE(IsSessionRenameErrorMessageDisplayed());
+  base::ListValue expected;
+  expected.GetList().push_back(base::Value("2"));
+  expected.GetList().push_back(base::Value("1"));
+  expected.GetList().push_back(base::Value("4"));
+  EXPECT_EQ(expected, *ExtractSessionsList());
+
+  // Check that the current session can be renamed properly.
+  RenameSession("2", "5");
+  EXPECT_FALSE(IsSessionRenameErrorMessageDisplayed());
+  expected.GetList()[0] = base::Value("5");
+  EXPECT_EQ(expected, *ExtractSessionsList());
+}
+
+IN_PROC_BROWSER_TEST_F(PolicyToolUITest, RenameSessionWithExistingSessionName) {
+  CreateMultipleSessionFiles(3);
+  ui_test_utils::NavigateToURL(browser(), GURL("chrome://policy-tool"));
+  EXPECT_EQ("2", ExtractSinglePolicyValue("SessionId"));
+
+  // Check that a session can not be renamed with a name of another existing
+  // session.
+  RenameSession("2", "1");
+  EXPECT_TRUE(IsSessionRenameErrorMessageDisplayed());
+  base::ListValue expected;
+  expected.GetList().push_back(base::Value("2"));
+  expected.GetList().push_back(base::Value("1"));
+  expected.GetList().push_back(base::Value("0"));
+  EXPECT_EQ(expected, *ExtractSessionsList());
+}
+
+IN_PROC_BROWSER_TEST_F(PolicyToolUITest, RenameSessionInvalidName) {
+  CreateMultipleSessionFiles(3);
+  ui_test_utils::NavigateToURL(browser(), GURL("chrome://policy-tool"));
+  EXPECT_EQ("2", ExtractSinglePolicyValue("SessionId"));
+
+  RenameSession("2", "../");
+  EXPECT_TRUE(IsSessionRenameErrorMessageDisplayed());
+  base::ListValue expected;
+  expected.GetList().push_back(base::Value("2"));
+  expected.GetList().push_back(base::Value("1"));
+  expected.GetList().push_back(base::Value("0"));
+  EXPECT_EQ(expected, *ExtractSessionsList());
+}

chrome/browser/ui/webui/policy_tool_ui_handler.cc

@@ -12,6 +12,8 @@
 #include "base/task_scheduler/post_task.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/strings/grit/components_strings.h"
+#include "ui/base/l10n/l10n_util.h"
 
 // static
 const base::FilePath::CharType PolicyToolUIHandler::kPolicyToolSessionsDir[] =
@@ -37,20 +39,29 @@ void PolicyToolUIHandler::RegisterMessages() {
       Profile::FromWebUI(web_ui())->GetPath().Append(kPolicyToolSessionsDir);
 
   web_ui()->RegisterMessageCallback(
-      "initialized", base::Bind(&PolicyToolUIHandler::HandleInitializedAdmin,
-                                base::Unretained(this)));
+      "initialized",
+      base::BindRepeating(&PolicyToolUIHandler::HandleInitializedAdmin,
+                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
-      "loadSession", base::Bind(&PolicyToolUIHandler::HandleLoadSession,
-                                base::Unretained(this)));
+      "loadSession",
+      base::BindRepeating(&PolicyToolUIHandler::HandleLoadSession,
+                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
-      "updateSession", base::Bind(&PolicyToolUIHandler::HandleUpdateSession,
-                                  base::Unretained(this)));
+      "renameSession",
+      base::BindRepeating(&PolicyToolUIHandler::HandleRenameSession,
+                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
-      "resetSession", base::Bind(&PolicyToolUIHandler::HandleResetSession,
-                                 base::Unretained(this)));
+      "updateSession",
+      base::BindRepeating(&PolicyToolUIHandler::HandleUpdateSession,
+                          base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
-      "deleteSession", base::Bind(&PolicyToolUIHandler::HandleDeleteSession,
-                                  base::Unretained(this)));
+      "resetSession",
+      base::BindRepeating(&PolicyToolUIHandler::HandleResetSession,
+                          base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "deleteSession",
+      base::BindRepeating(&PolicyToolUIHandler::HandleDeleteSession,
+                          base::Unretained(this)));
 }
 
 void PolicyToolUIHandler::OnJavascriptDisallowed() {
@@ -165,6 +176,8 @@ void PolicyToolUIHandler::OnFileRead(const std::string& contents) {
     // TODO(urusant): convert the policy values so that the types are
     // consistent with actual policy types.
     CallJavascriptFunction("policy.Page.setPolicyValues", *value);
+    CallJavascriptFunction("policy.Page.setSessionTitle",
+                           base::Value(session_name_));
     base::PostTaskWithTraitsAndReplyWithResult(
         FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
         base::BindOnce(&PolicyToolUIHandler::GetSessionsList,
@@ -215,6 +228,79 @@ void PolicyToolUIHandler::HandleLoadSession(const base::ListValue* args) {
   ImportFile();
 }
 
+PolicyToolUIHandler::SessionErrors PolicyToolUIHandler::DoRenameSession(
+    const base::FilePath::StringType& old_session_name,
+    const base::FilePath::StringType& new_session_name) {
+  const base::FilePath old_session_path = GetSessionPath(old_session_name);
+  const base::FilePath new_session_path = GetSessionPath(new_session_name);
+
+  // Check if the session files exist. If |old_session_name| doesn't exist, it
+  // means that is not a valid session. If |new_session_name| exists, it means
+  // that we can't do the rename because that will cause a file overwrite.
+  if (!PathExists(old_session_path))
+    return SessionErrors::kSessionNameNotExist;
+  if (PathExists(new_session_path))
+    return SessionErrors::kSessionNameExist;
+  if (!base::Move(old_session_path, new_session_path))
+    return SessionErrors::kRenamedSessionError;
+  return SessionErrors::kNone;
+}
+
+void PolicyToolUIHandler::OnSessionRenamed(
+    PolicyToolUIHandler::SessionErrors result) {
+  if (result == SessionErrors::kNone) {
+    base::PostTaskWithTraitsAndReplyWithResult(
+        FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
+        base::BindOnce(&PolicyToolUIHandler::GetSessionsList,
+                       base::Unretained(this)),
+        base::BindOnce(&PolicyToolUIHandler::OnSessionsListReceived,
+                       callback_weak_ptr_factory_.GetWeakPtr()));
+    CallJavascriptFunction("policy.Page.closeRenameSessionDialog");
+    return;
+  }
+  int error_message_id;
+  if (result == SessionErrors::kSessionNameNotExist) {
+    error_message_id = IDS_POLICY_TOOL_SESSION_NOT_EXIST;
+  } else if (result == SessionErrors::kSessionNameExist) {
+    error_message_id = IDS_POLICY_TOOL_SESSION_EXIST;
+  } else {
+    error_message_id = IDS_POLICY_TOOL_RENAME_FAILED;
+  }
+
+  CallJavascriptFunction(
+      "policy.Page.showRenameSessionError",
+      base::Value(l10n_util::GetStringUTF16(error_message_id)));
+}
+
+void PolicyToolUIHandler::HandleRenameSession(const base::ListValue* args) {
+  DCHECK_EQ(2U, args->GetSize());
+  base::FilePath::StringType old_session_name, new_session_name;
+  old_session_name =
+      base::FilePath::FromUTF8Unsafe(args->GetList()[0].GetString()).value();
+  new_session_name =
+      base::FilePath::FromUTF8Unsafe(args->GetList()[1].GetString()).value();
+
+  if (!IsValidSessionName(new_session_name) ||
+      !IsValidSessionName(old_session_name)) {
+    CallJavascriptFunction("policy.Page.showRenameSessionError",
+                           base::Value(l10n_util::GetStringUTF16(
+                               IDS_POLICY_TOOL_INVALID_SESSION_NAME)));
+    return;
+  }
+
+  // This is important in case the user renames the current active session.
+  // If we don't clear the current session name, after the rename, a new file
+  // will be created with the old name and with an empty dictionary in it.
+  session_name_.clear();
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
+      base::BindOnce(&PolicyToolUIHandler::DoRenameSession,
+                     base::Unretained(this), old_session_name,
+                     new_session_name),
+      base::BindOnce(&PolicyToolUIHandler::OnSessionRenamed,
+                     callback_weak_ptr_factory_.GetWeakPtr()));
+}
+
 bool PolicyToolUIHandler::DoUpdateSession(const std::string& contents) {
   // Sanity check that contents is not too big. Otherwise, passing it to
   // WriteFile will be int overflow.

chrome/browser/ui/webui/policy_tool_ui_handler.h

@@ -20,6 +20,14 @@ class PolicyToolUIHandler : public PolicyUIHandler {
  private:
   friend class PolicyToolUITest;
 
+  enum class SessionErrors {
+    kNone = 0,
+    kInvalidSessionName,
+    kSessionNameExist,
+    kSessionNameNotExist,
+    kRenamedSessionError,
+  };
+
   static const base::FilePath::CharType kPolicyToolSessionsDir[];
   static const base::FilePath::CharType kPolicyToolDefaultSessionName[];
   static const base::FilePath::CharType kPolicyToolSessionExtension[];
@@ -28,9 +36,16 @@ class PolicyToolUIHandler : public PolicyUIHandler {
   void ImportFile();
 
   void HandleInitializedAdmin(const base::ListValue* args);
+
   void HandleLoadSession(const base::ListValue* args);
+
+  // Rename a session if the new session name doesn't exist.
+  void HandleRenameSession(const base::ListValue* args);
+
   void HandleUpdateSession(const base::ListValue* args);
+
   void HandleResetSession(const base::ListValue* args);
+
   void HandleDeleteSession(const base::ListValue* args);
 
   void OnSessionDeleted(bool is_successful);
@@ -38,7 +53,14 @@ class PolicyToolUIHandler : public PolicyUIHandler {
   std::string ReadOrCreateFileCallback();
   void OnFileRead(const std::string& contents);
 
+  SessionErrors DoRenameSession(
+      const base::FilePath::StringType& old_session_name,
+      const base::FilePath::StringType& new_session_name);
+
+  void OnSessionRenamed(SessionErrors result);
+
   bool DoUpdateSession(const std::string& contents);
+
   void OnSessionUpdated(bool is_successful);
 
   bool IsValidSessionName(const base::FilePath::StringType& name) const;

components/policy_strings.grdp

@@ -228,6 +228,12 @@
   </message>
 
   <!-- chrome://policy-tool -->
+  <message name="IDS_POLICY_TOOL_CANCEL_RENAME" desc="Button to cancel the dialog that appears in rename session.">
+    Cancel
+  </message>
+  <message name="IDS_POLICY_TOOL_CONFIRM_RENAME" desc="Button that confirm a rename session.">
+    Confirm
+  </message>
   <message name="IDS_POLICY_TOOL_TITLE" desc="Admin page title and the title of the section that lists policies.">
     Policy management
   </message>
@@ -240,6 +246,12 @@
   <message name="IDS_POLICY_TOOL_LOAD_SESSION" desc="Label for the button that loads specified session.">
     Load session
   </message>
+  <message name="IDS_POLICY_TOOL_REMOVE_SESSION" desc="Label for the button that removes specified session.">
+    Remove
+  </message>
+  <message name="IDS_POLICY_TOOL_RENAME_SESSION" desc="Label for the button that renames specified session.">
+    Rename
+  </message>
   <message name="IDS_POLICY_TOOL_SESSION_NAME_PLACEHOLDER" desc="Placeholder for the input field that lets the user change policy management sessions.">
     Session name
   </message>
@@ -258,6 +270,18 @@
   <message name="IDS_POLICY_TOOL_DELETE_FAILED" desc="A message that is shown to the user when deleting the session fails because the session name that is sent from Javascript is invalid.">
     The session with this name is not valid for deletion.
   </message>
+  <message name="IDS_POLICY_TOOL_SESSION_NOT_EXIST" desc="A message that is shown to the user when
+    the selected session name does not exist.">
+    The selected session does not exist.
+  </message>
+  <message name="IDS_POLICY_TOOL_SESSION_EXIST" desc="A message that is show to the user when
+    there is another session with the same name in a renaming session.">
+    This session name already exists.
+  </message>
+  <message name="IDS_POLICY_TOOL_RENAME_FAILED" desc="A message that is shown to the user when
+    renaming the session fails because the function that rename the file failed.">
+    Failed to rename the session.
+  </message>
   <!-- chrome://policy -->
   <message name="IDS_POLICY_TITLE" desc="Page title and the title of the section that lists policies.">
     Policies