Commit badb017d authored by peter@chromium.org's avatar peter@chromium.org

Revert of SegmentedString::push() should always push a char in front (patchset...

Revert of SegmentedString::push() should always push a char in front (patchset #2 id:20001 of https://codereview.chromium.org/1308573006/ )

Reason for revert:
HTMLEntityParserTest.ConsumeHTMLEntityIncomplete fails on two Win bots

The newly added test is failing on WebKit Win7 (dbg) and WebKit Win Oilpan
(dbg) with the following message:

[ RUN      ] HTMLEntityParserTest.ConsumeHTMLEntityIncomplete
ASSERTION FAILED: !notEnoughCharacters

Sorry for the revert! Full output @ http://goo.gl/tnYH67

Original issue's description:
> SegmentedString::push() should always push a char in front
> 
> Before this CL, SegmentedString::push() had an exotic behavior, where
> two consecutive push would swap its order.
> 
> This CL changes the push() implementation so that it would always push
> the new char in front of the SegmentedString.
> This CL also updates the HTMLEntityParser code which relied on the behavior.
> 
> BUG=None
> TEST={SegmentedStringTest,HTMLEntityParserTest}.*
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201294

TBR=tasak@google.com,tzik@chromium.org,tkent@chromium.org,yosin@chromium.org,kouhei@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None

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

git-svn-id: svn://svn.chromium.org/blink/trunk@201318 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent f713507e
......@@ -3814,7 +3814,6 @@
'html/TimeRangesTest.cpp',
'html/canvas/CanvasFontCacheTest.cpp',
'html/forms/FileInputTypeTest.cpp',
'html/parser/HTMLEntityParserTest.cpp',
'html/parser/HTMLParserThreadTest.cpp',
'html/parser/HTMLPreloadScannerTest.cpp',
'html/parser/HTMLResourcePreloaderTest.cpp',
......
......@@ -95,8 +95,8 @@ static void unconsumeCharacters(SegmentedString& source, ConsumedCharacterBuffer
if (consumedCharacters.size() == 1)
source.push(consumedCharacters[0]);
else if (consumedCharacters.size() == 2) {
source.push(consumedCharacters[1]);
source.push(consumedCharacters[0]);
source.push(consumedCharacters[1]);
} else
source.prepend(SegmentedString(String(consumedCharacters)));
}
......@@ -212,8 +212,8 @@ bool consumeHTMLEntity(SegmentedString& source, DecodedHTMLEntity& decodedEntity
entityState = Hex;
continue;
}
source.push('x');
source.push('#');
source.push('x');
return false;
}
case MaybeHexUpperCaseX: {
......@@ -221,8 +221,8 @@ bool consumeHTMLEntity(SegmentedString& source, DecodedHTMLEntity& decodedEntity
entityState = Hex;
continue;
}
source.push('X');
source.push('#');
source.push('X');
return false;
}
case Hex: {
......
......@@ -27,7 +27,6 @@
#ifndef HTMLEntityParser_h
#define HTMLEntityParser_h
#include "core/CoreExport.h"
#include "platform/text/SegmentedString.h"
#include "wtf/Allocator.h"
......@@ -64,11 +63,11 @@ public:
UChar data[kMaxLength];
};
CORE_EXPORT bool consumeHTMLEntity(SegmentedString&, DecodedHTMLEntity& decodedEntity, bool& notEnoughCharacters, UChar additionalAllowedCharacter = '\0');
bool consumeHTMLEntity(SegmentedString&, DecodedHTMLEntity& decodedEntity, bool& notEnoughCharacters, UChar additionalAllowedCharacter = '\0');
// Used by the XML parser. Not suitable for use in HTML parsing. Use consumeHTMLEntity instead.
size_t decodeNamedEntityToUCharArray(const char*, UChar result[4]);
} // namespace blink
}
#endif
// Copyright 2015 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/parser/HTMLEntityParser.h"
#include <gtest/gtest.h>
namespace blink {
TEST(HTMLEntityParserTest, ConsumeHTMLEntityIncomplete)
{
String original("am"); // Incomplete by purpose.
SegmentedString src(original);
DecodedHTMLEntity entity;
bool notEnoughCharacters;
bool success = consumeHTMLEntity(src, entity, notEnoughCharacters);
EXPECT_TRUE(notEnoughCharacters);
EXPECT_FALSE(success);
// consumeHTMLEntity should recover the original SegmentedString state if failed.
EXPECT_STREQ(original.ascii().data(), src.toString().ascii().data());
}
} // namespace blink
......@@ -190,15 +190,13 @@ public:
void push(UChar c)
{
ASSERT(c);
if (!m_pushedChar1) {
m_currentChar = m_pushedChar1 = c;
m_pushedChar1 = c;
m_currentChar = m_pushedChar1 ? m_pushedChar1 : m_currentString.getCurrentChar();
updateSlowCaseFunctionPointers();
} else {
ASSERT(!m_pushedChar2);
m_pushedChar2 = m_pushedChar1;
m_currentChar = m_pushedChar1 = c;
m_pushedChar2 = c;
}
}
......
......@@ -47,18 +47,20 @@ TEST(SegmentedStringTest, CurrentChar)
EXPECT_EQ('c', copied.currentChar());
EXPECT_EQ('c', assigned.currentChar());
}
original.push('b');
original.push('a');
{
SegmentedString copied(original);
SegmentedString assigned;
assigned = original;
EXPECT_EQ("bcde", original.toString());
EXPECT_EQ('b', original.currentChar());
EXPECT_EQ('b', copied.currentChar());
EXPECT_EQ('b', assigned.currentChar());
EXPECT_EQ("acde", original.toString());
EXPECT_EQ('a', original.currentChar());
EXPECT_EQ('a', copied.currentChar());
EXPECT_EQ('a', assigned.currentChar());
}
original.push('a');
original.push('b');
{
// Two consecutive push means to push the second char *after* the
// first char in SegmentedString.
SegmentedString copied(original);
SegmentedString assigned;
assigned = original;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment