Bug 1170246 - Improving nsGlobalWindow::ConvertDialogOptions and adding some gTests, r=ehsan
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 17 Jun 2015 12:17:33 +0100
changeset 249335 3b73bf339093cd9e1c43b6b6c3b5750ca27297d0
parent 249334 6961dc3eba19885d44e16d28d908cdbf8ca98c44
child 249336 d8783629750506b806611ea15c368771fb847f33
push id28923
push userryanvm@gmail.com
push dateWed, 17 Jun 2015 18:57:11 +0000
treeherdermozilla-central@099d6cd6725e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1170246
milestone41.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1170246 - Improving nsGlobalWindow::ConvertDialogOptions and adding some gTests, r=ehsan
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
dom/base/test/gtest/TestParserDialogOptions.cpp
dom/base/test/gtest/moz.build
dom/base/test/moz.build
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -8857,90 +8857,92 @@ nsGlobalWindow::GetRealFrameElement(nsID
   ErrorResult rv;
   nsCOMPtr<nsIDOMElement> frameElement =
     do_QueryInterface(GetRealFrameElement(rv));
   frameElement.forget(aFrameElement);
 
   return rv.StealNSResult();
 }
 
+/* static */ bool
+nsGlobalWindow::TokenizeDialogOptions(nsAString& aToken,
+                                      nsAString::const_iterator& aIter,
+                                      nsAString::const_iterator aEnd)
+{
+  while (aIter != aEnd && nsCRT::IsAsciiSpace(*aIter)) {
+    ++aIter;
+  }
+
+  if (aIter == aEnd) {
+    return false;
+  }
+
+  if (*aIter == ';' || *aIter == ':' || *aIter == '=') {
+    aToken.Assign(*aIter);
+    ++aIter;
+    return true;
+  }
+
+  nsAString::const_iterator start = aIter;
+
+  // Skip characters until we find whitespace, ';', ':', or '='
+  while (aIter != aEnd && !nsCRT::IsAsciiSpace(*aIter) &&
+         *aIter != ';' &&
+         *aIter != ':' &&
+         *aIter != '=') {
+    ++aIter;
+  }
+
+  aToken.Assign(Substring(start, aIter));
+  return true;
+}
+
 // Helper for converting window.showModalDialog() options (list of ';'
 // separated name (:|=) value pairs) to a format that's parsable by
 // our normal window opening code.
 
-void
-ConvertDialogOptions(const nsAString& aOptions, nsAString& aResult)
+/* static */
+void
+nsGlobalWindow::ConvertDialogOptions(const nsAString& aOptions,
+                                     nsAString& aResult)
 {
   nsAString::const_iterator end;
   aOptions.EndReading(end);
 
   nsAString::const_iterator iter;
   aOptions.BeginReading(iter);
 
-  while (iter != end) {
-    // Skip whitespace.
-    while (nsCRT::IsAsciiSpace(*iter) && iter != end) {
-      ++iter;
-    }
-
-    nsAString::const_iterator name_start = iter;
-
-    // Skip characters until we find whitespace, ';', ':', or '='
-    while (iter != end && !nsCRT::IsAsciiSpace(*iter) &&
-           *iter != ';' &&
-           *iter != ':' &&
-           *iter != '=') {
-      ++iter;
-    }
-
-    nsAString::const_iterator name_end = iter;
-
-    // Skip whitespace.
-    while (nsCRT::IsAsciiSpace(*iter) && iter != end) {
-      ++iter;
-    }
-
-    if (*iter == ';') {
-      // No value found, skip the ';' and keep going.
-      ++iter;
-
+  nsAutoString token;
+  nsAutoString name;
+  nsAutoString value;
+
+  while (true) {
+    if (!TokenizeDialogOptions(name, iter, end)) {
+      break;
+    }
+
+    // Invalid name.
+    if (name.EqualsLiteral("=") ||
+        name.EqualsLiteral(":") ||
+        name.EqualsLiteral(";")) {
+      break;
+    }
+
+    if (!TokenizeDialogOptions(token, iter, end)) {
+      break;
+    }
+
+    if (!token.EqualsLiteral(":") && !token.EqualsLiteral("=")) {
       continue;
     }
 
-    nsAString::const_iterator value_start = iter;
-    nsAString::const_iterator value_end = iter;
-
-    if (*iter == ':' || *iter == '=') {
-      // We found name followed by ':' or '='. Look for a value.
-
-      iter++; // Skip the ':' or '='
-
-      // Skip whitespace.
-      while (nsCRT::IsAsciiSpace(*iter) && iter != end) {
-        ++iter;
-      }
-
-      value_start = iter;
-
-      // Skip until we find whitespace, or ';'.
-      while (iter != end && !nsCRT::IsAsciiSpace(*iter) &&
-             *iter != ';') {
-        ++iter;
-      }
-
-      value_end = iter;
-
-      // Skip whitespace.
-      while (nsCRT::IsAsciiSpace(*iter) && iter != end) {
-        ++iter;
-      }
-    }
-
-    const nsDependentSubstring& name = Substring(name_start, name_end);
-    const nsDependentSubstring& value = Substring(value_start, value_end);
+    // We found name followed by ':' or '='. Look for a value.
+    if (!TokenizeDialogOptions(value, iter, end)) {
+      break;
+    }
 
     if (name.LowerCaseEqualsLiteral("center")) {
       if (value.LowerCaseEqualsLiteral("on")  ||
           value.LowerCaseEqualsLiteral("yes") ||
           value.LowerCaseEqualsLiteral("1")) {
         aResult.AppendLiteral(",centerscreen=1");
       }
     } else if (name.LowerCaseEqualsLiteral("dialogwidth")) {
@@ -8972,21 +8974,21 @@ ConvertDialogOptions(const nsAString& aO
     } else if (name.LowerCaseEqualsLiteral("scroll")) {
       if (value.LowerCaseEqualsLiteral("off")  ||
           value.LowerCaseEqualsLiteral("no") ||
           value.LowerCaseEqualsLiteral("0")) {
         aResult.AppendLiteral(",scrollbars=0");
       }
     }
 
-    if (iter == end) {
+    if (iter == end ||
+        !TokenizeDialogOptions(token, iter, end) ||
+        !token.EqualsLiteral(";")) {
       break;
     }
-
-    iter++;
   }
 }
 
 already_AddRefed<nsIVariant>
 nsGlobalWindow::ShowModalDialog(const nsAString& aUrl, nsIVariant* aArgument,
                                 const nsAString& aOptions, ErrorResult& aError)
 {
   if (mDoc) {
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -905,16 +905,23 @@ public:
   nsIDOMOfflineResourceList* GetApplicationCache(mozilla::ErrorResult& aError);
 
   mozilla::dom::Console* GetConsole(mozilla::ErrorResult& aRv);
 
   void GetSidebar(mozilla::dom::OwningExternalOrWindowProxy& aResult,
                   mozilla::ErrorResult& aRv);
   already_AddRefed<mozilla::dom::External> GetExternal(mozilla::ErrorResult& aRv);
 
+  // Exposed only for testing
+  static bool
+  TokenizeDialogOptions(nsAString& aToken, nsAString::const_iterator& aIter,
+                        nsAString::const_iterator aEnd);
+  static void
+  ConvertDialogOptions(const nsAString& aOptions, nsAString& aResult);
+
 protected:
   bool AlertOrConfirm(bool aAlert, const nsAString& aMessage,
                       mozilla::ErrorResult& aError);
 
 public:
   void Alert(mozilla::ErrorResult& aError);
   void Alert(const nsAString& aMessage, mozilla::ErrorResult& aError);
   already_AddRefed<mozilla::dom::cache::CacheStorage> GetCaches(mozilla::ErrorResult& aRv);
new file mode 100644
--- /dev/null
+++ b/dom/base/test/gtest/TestParserDialogOptions.cpp
@@ -0,0 +1,138 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "gtest/gtest.h"
+#include "nsGlobalWindow.h"
+
+struct dialog_test {
+  const char* input;
+  const char* output;
+};
+
+void runTokenizeTest(dialog_test& test)
+{
+  NS_ConvertUTF8toUTF16 input(test.input);
+
+  nsAString::const_iterator end;
+  input.EndReading(end);
+
+  nsAString::const_iterator iter;
+  input.BeginReading(iter);
+
+  nsAutoString result;
+  nsAutoString token;
+
+  while (nsGlobalWindow::TokenizeDialogOptions(token, iter, end)) {
+    if (!result.IsEmpty()) {
+      result.Append(',');
+    }
+
+    result.Append(token);
+  }
+
+  ASSERT_STREQ(test.output, NS_ConvertUTF16toUTF8(result).get()) << "Testing " << test.input;
+}
+
+void runTest(dialog_test& test)
+{
+  NS_ConvertUTF8toUTF16 input(test.input);
+
+  nsAutoString result;
+  nsGlobalWindow::ConvertDialogOptions(input, result);
+
+  ASSERT_STREQ(test.output, NS_ConvertUTF16toUTF8(result).get()) << "Testing " << test.input;
+}
+
+TEST(GlobalWindowDialogOptions, TestDialogTokenize)
+{
+  dialog_test tests[] = {
+    /// Empty strings
+    { "", "" },
+    { " ", "" },
+    { "   ", "" },
+
+    // 1 token
+    { "a", "a" },
+    { " a", "a" },
+    { "  a  ", "a" },
+    { "aa", "aa" },
+    { " aa", "aa" },
+    { "  aa  ", "aa" },
+    { ";", ";" },
+    { ":", ":" },
+    { "=", "=" },
+
+    // 2 tokens
+    { "a=", "a,=" },
+    { "  a=  ", "a,=" },
+    { "  a  =  ", "a,=" },
+    { "aa=", "aa,=" },
+    { "  aa=  ", "aa,=" },
+    { "  aa  =  ", "aa,=" },
+    { ";= ", ";,=" },
+    { "==", "=,=" },
+    { "::", ":,:" },
+
+    // 3 tokens
+    { "a=2", "a,=,2" },
+    { "===", "=,=,=" },
+    { ";:=", ";,:,=" },
+
+    // more
+    { "aaa;bbb:ccc", "aaa,;,bbb,:,ccc" },
+
+    // sentinel
+    { nullptr, nullptr }
+  };
+
+  for (uint32_t i = 0; tests[i].input; ++i) {
+    runTokenizeTest(tests[i]);
+  }
+}
+TEST(GlobalWindowDialogOptions, TestDialogOptions)
+{
+  dialog_test tests[] = {
+    /// Empty strings
+    { "", "" },
+    { " ", "" },
+    { "   ", "" },
+
+    // Name without params
+    { "a", "" },
+    { " a", "" },
+    { "  a  ", "" },
+    { "a=", "" },
+    { "  a=  ", "" },
+    { "  a  =  ", "" },
+
+    // 1 unknown value
+    { "a=2", "" },
+    { " a=2 ", "" },
+    { "  a  =  2 ", "" },
+    { "a:2", "" },
+    { " a:2 ", "" },
+    { "  a  :  2 ", "" },
+
+    // 1 known value, wrong value
+    { "center=2", "" },
+    { "center:2", "" },
+
+    // 1 known value, good value
+    { "center=on", ",centerscreen=1" },
+    { "center:on", ",centerscreen=1" },
+    { " center : on ", ",centerscreen=1" },
+
+    // nonsense stuff
+    { " ; ", "" },
+
+    // sentinel
+    { nullptr, nullptr }
+  };
+
+  for (uint32_t i = 0; tests[i].input; ++i) {
+    runTest(tests[i]);
+  }
+}
new file mode 100644
--- /dev/null
+++ b/dom/base/test/gtest/moz.build
@@ -0,0 +1,17 @@
+# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
+# vim: set filetype=python:
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, you can obtain one at http://mozilla.org/MPL/2.0/.
+
+UNIFIED_SOURCES += [
+    'TestParserDialogOptions.cpp',
+]
+
+LOCAL_INCLUDES += [
+    '/dom/base'
+]
+
+include('/ipc/chromium/chromium-config.mozbuild')
+
+FINAL_LIBRARY = 'xul-gtest'
--- a/dom/base/test/moz.build
+++ b/dom/base/test/moz.build
@@ -31,8 +31,12 @@ if CONFIG['MOZ_CHILD_PERMISSIONS']:
 MOCHITEST_CHROME_MANIFESTS += [
     'chrome.ini',
     'chrome/chrome.ini',
 ]
 
 BROWSER_CHROME_MANIFESTS += [
     'browser.ini',
 ]
+
+TEST_DIRS += [
+    'gtest',
+]