[Binding] [Refactoring] Make SerializedScriptValueWriter protected (issue 2017543002 by peria@chromium.org)

[pe...@chromium.org]

Reviewers: Yuki, haraken
CL: https://codereview.chromium.org/2017543002/

Message:
PTL

Description:
Make most methods of SerializedScriptValueWriter protected.
We still have a usage of takeWireString(), and it will be
removed in another CL.


BUG=614919

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+19, -29 lines):
M third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
M third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
M third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
M third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.h
M third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.cpp
M third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.h
M third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp


Index: third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
index 21f16e3585fc7b16665bf770b98e53161faefab3..e2569ab776c2341a5698cc0d29c33ab011edf733 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp
@@ -762,6 +762,14 @@ ScriptValueSerializer::Status ScriptValueSerializer::serialize(v8::Local<v8::Val
return m_status;
}

+// static
+String ScriptValueSerializer::serializeWTFString(const String& data)
+{
+ SerializedScriptValueWriter valueWriter;
+ valueWriter.writeWebCoreString(data);
+ return valueWriter.takeWireString();
+}
+
ScriptValueSerializer::StateBase* ScriptValueSerializer::doSerialize(v8::Local<v8::Value> value, ScriptValueSerializer::StateBase* next)
{
m_writer.writeReferenceCount(m_nextObjectReference);
Index: third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
index 3f43661643ae6fb28c0ad5615433b578359ee9df..696ddfb8682c00534e3cda2a293dbcf9f04b4d30 100644
--- a/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
+++ b/third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.h
@@ -110,8 +110,12 @@ public:
{
}

- // Write functions for primitive types.
+ String takeWireString();
+
+protected:
+ friend class ScriptValueSerializer;

+ // Write functions for primitive types.
void writeUndefined();
void writeNull();
void writeTrue();
@@ -149,7 +153,6 @@ public:
void writeObject(uint32_t numProperties);
void writeSparseArray(uint32_t numProperties, uint32_t length);
void writeDenseArray(uint32_t numProperties, uint32_t length);
- String takeWireString();
void writeReferenceCount(uint32_t numberOfReferences);
void writeGenerateFreshObject();
void writeGenerateFreshSparseArray(uint32_t length);
@@ -159,7 +162,6 @@ public:
void writeMap(uint32_t length);
void writeSet(uint32_t length);

-protected:
void doWriteFile(const File&);
void doWriteArrayBuffer(const DOMArrayBuffer&);
void doWriteString(const char* data, int length);
@@ -216,6 +218,8 @@ public:
Status serialize(v8::Local<v8::Value>);
String errorMessage() { return m_errorMessage; }

+ static String serializeWTFString(const String&);
+
protected:
class StateBase {
USING_FAST_MALLOC(StateBase);
Index: third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
index 9af8a7808228d9e8c3bcbba3452c4e762254b06b..b22c320d7468d1e206e0ea73d4c0d48abe72758d 100644
--- a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
@@ -76,10 +76,8 @@ SerializedScriptValue::~SerializedScriptValue()

PassRefPtr<SerializedScriptValue> SerializedScriptValue::nullValue()
{
- SerializedScriptValueWriter writer;
- writer.writeNull();
- String wireData = writer.takeWireString();
- return adoptRef(new SerializedScriptValue(wireData));
+ v8::Local<v8::Value> value;
+ return SerializedScriptValueFactory::instance().create(v8::Isolate::GetCurrent(), value, nullptr, ASSERT_NO_EXCEPTION);
}

// Convert serialized string to big endian wire data.
Index: third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.cpp b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.cpp
index 03781b5bb7b99a4813d8b4ef5fa012443a2508b8..5a1527640ed465f092e87ec47e305c15d76a2923 100644
--- a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.cpp
@@ -91,16 +91,8 @@ PassRefPtr<SerializedScriptValue> SerializedScriptValueFactory::createFromWireBy

PassRefPtr<SerializedScriptValue> SerializedScriptValueFactory::create(const String& data)
{
- return create(v8::Isolate::GetCurrent(), data);
-}
-
-PassRefPtr<SerializedScriptValue> SerializedScriptValueFactory::create(v8::Isolate* isolate, const String& data)
-{
- ASSERT_NOT_REACHED();
- SerializedScriptValueWriter writer;
- writer.writeWebCoreString(data);
- String wireData = writer.takeWireString();
- return createFromWire(wireData);
+ // This method is expected to be called iff |this| is SerializedScriptValueFactoryForModules.
+ return createFromWire(ScriptValueSerializer::serializeWTFString(data));
}

PassRefPtr<SerializedScriptValue> SerializedScriptValueFactory::create()
Index: third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.h b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.h
index 0abe1a34422138e99d13b3715cdb236435633d98..67b7182cd91a8c4a911ced6e4ffe1b7fa20e59b9 100644
--- a/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.h
+++ b/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValueFactory.h
@@ -30,7 +30,6 @@ public:
PassRefPtr<SerializedScriptValue> createFromWire(const String&);
PassRefPtr<SerializedScriptValue> createFromWireBytes(const char* data, size_t length);
PassRefPtr<SerializedScriptValue> create(const String&);
- virtual PassRefPtr<SerializedScriptValue> create(v8::Isolate*, const String&);
PassRefPtr<SerializedScriptValue> create();
PassRefPtr<SerializedScriptValue> create(v8::Isolate*, const ScriptValue&, WebBlobInfoArray*, ExceptionState&);

Index: third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp
diff --git a/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp b/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp
index 5f03c5b3ee6a21c1a8de76b4e91d75bcc2b27bca..dfad6bf695f3cc01371b85b4b3f51f1fa8d5195f 100644
--- a/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp
+++ b/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.cpp
@@ -17,14 +17,6 @@ PassRefPtr<SerializedScriptValue> SerializedScriptValueForModulesFactory::create
return SerializedScriptValueFactory::create(isolate, value, writer, transferables, blobInfo, exceptionState);
}

-PassRefPtr<SerializedScriptValue> SerializedScriptValueForModulesFactory::create(v8::Isolate* isolate, const String& data)
-{
- SerializedScriptValueWriterForModules writer;
- writer.writeWebCoreString(data);
- String wireData = writer.takeWireString();
- return createFromWire(wireData);
-}
-
ScriptValueSerializer::Status SerializedScriptValueForModulesFactory::doSerialize(v8::Local<v8::Value> value, SerializedScriptValueWriter& writer, Transferables* transferables, WebBlobInfoArray* blobInfo, BlobDataHandleMap& blobDataHandles, v8::TryCatch& tryCatch, String& errorMessage, v8::Isolate* isolate)
{
ScriptValueSerializerForModules serializer(writer, transferables, blobInfo, blobDataHandles, tryCatch, ScriptState::current(isolate));
@@ -49,4 +41,3 @@ v8::Local<v8::Value> SerializedScriptValueForModulesFactory::deserialize(String&
}

} // namespace blink
-
Index: third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.h
diff --git a/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.h b/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.h
index b133793e484c030686d9dfcd6952ffca0a7397bc..71e9d937cf5ec02b92cc1cf531104800b85e190c 100644
--- a/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.h
+++ b/third_party/WebKit/Source/bindings/modules/v8/SerializedScriptValueForModulesFactory.h
@@ -17,7 +17,6 @@ public:
SerializedScriptValueForModulesFactory() : SerializedScriptValueFactory() { }

PassRefPtr<SerializedScriptValue> create(v8::Isolate*, v8::Local<v8::Value>, Transferables*, WebBlobInfoArray*, ExceptionState&) override;
- PassRefPtr<SerializedScriptValue> create(v8::Isolate*, const String&) override;

protected:
ScriptValueSerializer::Status doSerialize(v8::Local<v8::Value>, SerializedScriptValueWriter&, Transferables*, WebBlobInfoArray*, BlobDataHandleMap&, v8::TryCatch&, String& errorMessage, v8::Isolate*) override;
@@ -28,4 +27,3 @@ protected:
} // namespace blink

#endif // SerializedScriptValueForModulesFactory_h
-

[har...@chromium.org]

LGTM


https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/2017543002/1
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/2017543002/1


https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

Try jobs failed on following builders:
linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/77087)

https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

[commit-bot@chromium.org via codereview.chromium.org]

Try jobs failed on following builders:

win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/220148)

https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

[commit-bot@chromium.org via codereview.chromium.org]

Try jobs failed on following builders:

win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/builds/154426)

https://codereview.chromium.org/2017543002/

[pe...@chromium.org]

PS1 was broken, because it uses an isolate with an empty context to create
ScriptValueSerializer.
PS2 is just a workaround. I'll redesign of creating SeraizedScriptValue
instances in another CL and update this CL.

https://codereview.chromium.org/2017543002/

[pe...@chromium.org]

PTAL.

SerializedScriptValueFactory still uses writer.takeWireString(),
and removing it will be a bit complicated to do in one CL,
so let me do it in another CL.

https://codereview.chromium.org/2017543002/

[har...@chromium.org]

LGTM


https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at

https://chromium-cq-status.appspot.com/patch-status/2017543002/60001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/2017543002/60001


https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #4 (id:60001)

https://codereview.chromium.org/2017543002/

[commit-bot@chromium.org via codereview.chromium.org]

Patchset 4 (id:??) landed as
https://crrev.com/6b136d3aa98f994b1d540d4dc95fdc2b3daee005
Cr-Commit-Position: refs/heads/master@{#396681}

https://codereview.chromium.org/2017543002/