Revert "Revert of HTMLSelectElement does not include selected index/i…

…ndices while saving state (patchset #8 id:160001 of https://codereview.chromium.org/541693003/)"

Reason for revert: The actual cause of the test failure was the lack of support for localized strings in test framework for <select>. That was fixed with https://codereview.chromium.org/593113003/

BUG=401506

Review URL: https://codereview.chromium.org/603383002

git-svn-id: svn://svn.chromium.org/blink/trunk@182741 bbb929c8-8fbe-4397-9dbb-9b2b20218538

Source/core/core.gypi

@@ -3556,6 +3556,7 @@
             'html/HTMLDimensionTest.cpp',
             'html/HTMLFormControlElementTest.cpp',
             'html/HTMLLinkElementSizesAttributeTest.cpp',
+            'html/HTMLSelectElementTest.cpp',
             'html/HTMLTextFormControlElementTest.cpp',
             'html/LinkRelAttributeTest.cpp',
             'html/TimeRangesTest.cpp',

Source/core/html/HTMLSelectElement.cpp

@@ -1040,6 +1040,7 @@ FormControlState HTMLSelectElement::saveFormControlState() const
         if (!option->selected())
             continue;
         state.append(option->value());
+        state.append(String::number(i));
         if (!multiple())
             break;
     }
@@ -1074,21 +1075,34 @@ void HTMLSelectElement::restoreFormControlState(const FormControlState& state)
         toHTMLOptionElement(items[i])->setSelectedState(false);
     }
 
+    // The saved state should have at least one value and an index.
+    ASSERT(state.valueSize() >= 2);
     if (!multiple()) {
-        size_t foundIndex = searchOptionsForValue(state[0], 0, itemsSize);
-        if (foundIndex != kNotFound)
-            toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
+        size_t index = state[1].toUInt();
+        if (index < itemsSize && isHTMLOptionElement(items[index]) && toHTMLOptionElement(items[index])->value() == state[0]) {
+            toHTMLOptionElement(items[index])->setSelectedState(true);
+        } else {
+            size_t foundIndex = searchOptionsForValue(state[0], 0, itemsSize);
+            if (foundIndex != kNotFound)
+                toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
+        }
     } else {
         size_t startIndex = 0;
-        for (size_t i = 0; i < state.valueSize(); ++i) {
+        for (size_t i = 0; i < state.valueSize(); i+= 2) {
             const String& value = state[i];
-            size_t foundIndex = searchOptionsForValue(value, startIndex, itemsSize);
-            if (foundIndex == kNotFound)
-                foundIndex = searchOptionsForValue(value, 0, startIndex);
-            if (foundIndex == kNotFound)
-                continue;
-            toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
-            startIndex = foundIndex + 1;
+            const size_t index = state[i + 1].toUInt();
+            if (index < itemsSize && isHTMLOptionElement(items[index]) && toHTMLOptionElement(items[index])->value() == value) {
+                toHTMLOptionElement(items[index])->setSelectedState(true);
+                startIndex = index + 1;
+            } else {
+                size_t foundIndex = searchOptionsForValue(value, startIndex, itemsSize);
+                if (foundIndex == kNotFound)
+                    foundIndex = searchOptionsForValue(value, 0, startIndex);
+                if (foundIndex == kNotFound)
+                    continue;
+                toHTMLOptionElement(items[foundIndex])->setSelectedState(true);
+                startIndex = foundIndex + 1;
+            }
         }
     }
 

Source/core/html/HTMLSelectElementTest.cpp

@@ -0,0 +1,105 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "core/html/HTMLSelectElement.h"
+
+#include "core/frame/FrameView.h"
+#include "core/html/HTMLDocument.h"
+#include "core/html/forms/FormController.h"
+#include "core/loader/EmptyClients.h"
+#include "core/testing/DummyPageHolder.h"
+#include <gtest/gtest.h>
+
+namespace blink {
+
+class HTMLSelectElementTest : public::testing::Test {
+protected:
+    virtual void SetUp() OVERRIDE;
+    HTMLDocument& document() const { return *m_document; }
+
+private:
+    OwnPtr<DummyPageHolder> m_dummyPageHolder;
+    RefPtrWillBePersistent<HTMLDocument> m_document;
+};
+
+void HTMLSelectElementTest::SetUp()
+{
+    Page::PageClients pageClients;
+    fillWithEmptyClients(pageClients);
+    m_dummyPageHolder = DummyPageHolder::create(IntSize(800, 600), &pageClients);
+
+    m_document = toHTMLDocument(&m_dummyPageHolder->document());
+    m_document->setMimeType("text/html");
+    m_document->setCharset("utf-8");
+}
+
+TEST_F(HTMLSelectElementTest, SaveRestoreSelectSingleFormControlState)
+{
+    document().documentElement()->setInnerHTML(String("<!DOCTYPE HTML><select id='sel'>"
+        "<option value='111' id='0'>111</option>"
+        "<option value='222'>222</option>"
+        "<option value='111' selected id='2'>!666</option>"
+        "<option value='999'>999</option></select>"), ASSERT_NO_EXCEPTION);
+    document().view()->updateLayoutAndStyleIfNeededRecursive();
+    Element* element = document().getElementById("sel");
+    HTMLFormControlElementWithState* select = toHTMLSelectElement(element);
+    HTMLOptionElement* opt0 = toHTMLOptionElement(document().getElementById("0"));
+    HTMLOptionElement* opt2 = toHTMLOptionElement(document().getElementById("2"));
+
+    // Save the select element state, and then restore again.
+    // Test passes if the restored state is not changed.
+    EXPECT_EQ(2, toHTMLSelectElement(element)->selectedIndex());
+    EXPECT_FALSE(opt0->selected());
+    EXPECT_TRUE(opt2->selected());
+    FormControlState selectState = select->saveFormControlState();
+    EXPECT_EQ(2U, selectState.valueSize());
+
+    // Clear the selected state, to be restored by restoreFormControlState.
+    toHTMLSelectElement(select)->setSelectedIndex(-1);
+    ASSERT_FALSE(opt2->selected());
+
+    // Restore
+    select->restoreFormControlState(selectState);
+    EXPECT_EQ(2, toHTMLSelectElement(element)->selectedIndex());
+    EXPECT_FALSE(opt0->selected());
+    EXPECT_TRUE(opt2->selected());
+}
+
+TEST_F(HTMLSelectElementTest, SaveRestoreSelectMultipleFormControlState)
+{
+    document().documentElement()->setInnerHTML(String("<!DOCTYPE HTML><select id='sel' multiple>"
+        "<option value='111' id='0'>111</option>"
+        "<option value='222'>222</option>"
+        "<option value='111' selected id='2'>!666</option>"
+        "<option value='999' selected id='3'>999</option></select>"), ASSERT_NO_EXCEPTION);
+    document().view()->updateLayoutAndStyleIfNeededRecursive();
+    HTMLFormControlElementWithState* select = toHTMLSelectElement(document().getElementById("sel"));
+
+    HTMLOptionElement* opt0 = toHTMLOptionElement(document().getElementById("0"));
+    HTMLOptionElement* opt2 = toHTMLOptionElement(document().getElementById("2"));
+    HTMLOptionElement* opt3 = toHTMLOptionElement(document().getElementById("3"));
+
+    // Save the select element state, and then restore again.
+    // Test passes if the selected options are not changed.
+    EXPECT_FALSE(opt0->selected());
+    EXPECT_TRUE(opt2->selected());
+    EXPECT_TRUE(opt3->selected());
+    FormControlState selectState = select->saveFormControlState();
+    EXPECT_EQ(4U, selectState.valueSize());
+
+    // Clear the selected state, to be restored by restoreFormControlState.
+    opt2->setSelected(false);
+    opt3->setSelected(false);
+    ASSERT_FALSE(opt2->selected());
+    ASSERT_FALSE(opt3->selected());
+
+    // Restore
+    select->restoreFormControlState(selectState);
+    EXPECT_FALSE(opt0->selected());
+    EXPECT_TRUE(opt2->selected());
+    EXPECT_TRUE(opt3->selected());
+}
+
+} // namespace blink

Source/core/html/forms/FormController.cpp

@@ -413,7 +413,7 @@ static String formStateSignature()
     // In the legacy version of serialized state, the first item was a name
     // attribute value of a form control. The following string literal should
     // contain some characters which are rarely used for name attribute values.
-    DEFINE_STATIC_LOCAL(String, signature, ("\n\r?% WebKit serialized form state version 8 \n\r=&"));
+    DEFINE_STATIC_LOCAL(String, signature, ("\n\r?% Blink serialized form state version 9 \n\r=&"));
     return signature;
 }