From 7429b91edafef18585d55ada62ba4ff01b88958c Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Mon, 18 Jul 2016 15:58:58 -0700 Subject: [PATCH] JavaScript: move extension binary info to separate struct. --- js/commonjs/export.js | 2 + js/message.js | 71 +++++++++++-------- .../protobuf/compiler/js/js_generator.cc | 50 ++++++++++--- 3 files changed, 85 insertions(+), 38 deletions(-) diff --git a/js/commonjs/export.js b/js/commonjs/export.js index a3cfbd6f1d..23ae47f9e1 100644 --- a/js/commonjs/export.js +++ b/js/commonjs/export.js @@ -9,12 +9,14 @@ goog.require('goog.object'); goog.require('jspb.BinaryReader'); goog.require('jspb.BinaryWriter'); goog.require('jspb.ExtensionFieldInfo'); +goog.require('jspb.ExtensionFieldBinaryInfo'); goog.require('jspb.Message'); exports.Message = jspb.Message; exports.BinaryReader = jspb.BinaryReader; exports.BinaryWriter = jspb.BinaryWriter; exports.ExtensionFieldInfo = jspb.ExtensionFieldInfo; +exports.ExtensionFieldBinaryInfo = jspb.ExtensionFieldBinaryInfo; // These are used by generated code but should not be used directly by clients. exports.exportSymbol = goog.exportSymbol; diff --git a/js/message.js b/js/message.js index e8185dee40..f746ee624b 100644 --- a/js/message.js +++ b/js/message.js @@ -35,6 +35,7 @@ */ goog.provide('jspb.ExtensionFieldInfo'); +goog.provide('jspb.ExtensionFieldBinaryInfo'); goog.provide('jspb.Message'); goog.require('goog.array'); @@ -84,19 +85,12 @@ goog.forwardDeclare('xid.String'); * @param {?function(new: jspb.Message, Array=)} ctor * @param {?function((boolean|undefined),!jspb.Message):!Object} toObjectFn * @param {number} isRepeated - * @param {?function(number,?)=} opt_binaryReaderFn - * @param {?function(number,?)|function(number,?,?,?,?,?)=} opt_binaryWriterFn - * @param {?function(?,?)=} opt_binaryMessageSerializeFn - * @param {?function(?,?)=} opt_binaryMessageDeserializeFn - * @param {?boolean=} opt_isPacked * @constructor * @struct * @template T */ jspb.ExtensionFieldInfo = function(fieldNumber, fieldName, ctor, toObjectFn, - isRepeated, opt_binaryReaderFn, opt_binaryWriterFn, - opt_binaryMessageSerializeFn, opt_binaryMessageDeserializeFn, - opt_isPacked) { + isRepeated) { /** @const */ this.fieldIndex = fieldNumber; /** @const */ @@ -106,20 +100,37 @@ jspb.ExtensionFieldInfo = function(fieldNumber, fieldName, ctor, toObjectFn, /** @const */ this.toObjectFn = toObjectFn; /** @const */ - this.binaryReaderFn = opt_binaryReaderFn; + this.isRepeated = isRepeated; +}; + +/** + * Stores binary-related information for a single extension field. + * @param {!jspb.ExtensionFieldInfo} fieldInfo + * @param {?function(number,?)=} binaryReaderFn + * @param {?function(number,?)|function(number,?,?,?,?,?)=} binaryWriterFn + * @param {?function(?,?)=} opt_binaryMessageSerializeFn + * @param {?function(?,?)=} opt_binaryMessageDeserializeFn + * @param {?boolean=} opt_isPacked + * @constructor + * @struct + * @template T + */ +jspb.ExtensionFieldBinaryInfo = function(fieldInfo, binaryReaderFn, binaryWriterFn, + binaryMessageSerializeFn, binaryMessageDeserializeFn, isPacked) { /** @const */ - this.binaryWriterFn = opt_binaryWriterFn; + this.fieldInfo = fieldInfo; /** @const */ - this.binaryMessageSerializeFn = opt_binaryMessageSerializeFn; + this.binaryReaderFn = binaryReaderFn; /** @const */ - this.binaryMessageDeserializeFn = opt_binaryMessageDeserializeFn; + this.binaryWriterFn = binaryWriterFn; /** @const */ - this.isRepeated = isRepeated; + this.binaryMessageSerializeFn = binaryMessageSerializeFn; /** @const */ - this.isPacked = opt_isPacked; + this.binaryMessageDeserializeFn = binaryMessageDeserializeFn; + /** @const */ + this.isPacked = isPacked; }; - /** * @return {boolean} Does this field represent a sub Message? */ @@ -491,11 +502,13 @@ jspb.Message.toObjectExtension = function(proto, obj, extensions, jspb.Message.serializeBinaryExtensions = function(proto, writer, extensions, getExtensionFn) { for (var fieldNumber in extensions) { - var fieldInfo = extensions[fieldNumber]; + var binaryFieldInfo = extensions[fieldNumber]; + var fieldInfo = binaryFieldInfo.fieldInfo; + // The old codegen doesn't add the extra fields to ExtensionFieldInfo, so we // need to gracefully error-out here rather than produce a null dereference // below. - if (!fieldInfo.binaryWriterFn) { + if (!binaryFieldInfo.binaryWriterFn) { throw new Error('Message extension present that was generated ' + 'without binary serialization support'); } @@ -508,16 +521,17 @@ jspb.Message.serializeBinaryExtensions = function(proto, writer, extensions, // message may require binary support, so we can *only* catch this error // here, at runtime (and this decoupled codegen is the whole point of // extensions!). - if (fieldInfo.binaryMessageSerializeFn) { - fieldInfo.binaryWriterFn.call(writer, fieldInfo.fieldIndex, - value, fieldInfo.binaryMessageSerializeFn); + if (binaryFieldInfo.binaryMessageSerializeFn) { + binaryFieldInfo.binaryWriterFn.call(writer, fieldInfo.fieldIndex, + value, binaryFieldInfo.binaryMessageSerializeFn); } else { throw new Error('Message extension present holding submessage ' + 'without binary support enabled, and message is ' + 'being serialized to binary format'); } } else { - fieldInfo.binaryWriterFn.call(writer, fieldInfo.fieldIndex, value); + binaryFieldInfo.binaryWriterFn.call( + writer, fieldInfo.fieldIndex, value); } } } @@ -535,12 +549,13 @@ jspb.Message.serializeBinaryExtensions = function(proto, writer, extensions, */ jspb.Message.readBinaryExtension = function(msg, reader, extensions, getExtensionFn, setExtensionFn) { - var fieldInfo = extensions[reader.getFieldNumber()]; - if (!fieldInfo) { + var binaryFieldInfo = extensions[reader.getFieldNumber()]; + var fieldInfo = binaryFieldInfo.fieldInfo; + if (!binaryFieldInfo) { reader.skipField(); return; } - if (!fieldInfo.binaryReaderFn) { + if (!binaryFieldInfo.binaryReaderFn) { throw new Error('Deserializing extension whose generated code does not ' + 'support binary format'); } @@ -548,14 +563,14 @@ jspb.Message.readBinaryExtension = function(msg, reader, extensions, var value; if (fieldInfo.isMessageType()) { value = new fieldInfo.ctor(); - fieldInfo.binaryReaderFn.call( - reader, value, fieldInfo.binaryMessageDeserializeFn); + binaryFieldInfo.binaryReaderFn.call( + reader, value, binaryFieldInfo.binaryMessageDeserializeFn); } else { // All other types. - value = fieldInfo.binaryReaderFn.call(reader); + value = binaryFieldInfo.binaryReaderFn.call(reader); } - if (fieldInfo.isRepeated && !fieldInfo.isPacked) { + if (fieldInfo.isRepeated && !binaryFieldInfo.isPacked) { var currentList = getExtensionFn.call(msg, fieldInfo); if (!currentList) { setExtensionFn.call(msg, fieldInfo, [value]); diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index e030066ec2..d3852a9593 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -2524,6 +2524,29 @@ void Generator::GenerateClassExtensionFieldInfo(const GeneratorOptions& options, "$class$.extensions = {};\n" "\n", "class", GetPath(options, desc)); + + if (options.binary) { + printer->Print( + "\n" + "/**\n" + " * The extensions registered with this message class. This is a " + "map of\n" + " * extension field number to fieldInfo object.\n" + " *\n" + " * For example:\n" + " * { 123: {fieldIndex: 123, fieldName: {my_field_name: 0}, " + "ctor: proto.example.MyMessage} }\n" + " *\n" + " * fieldName contains the JsCompiler renamed field name property " + "so that it\n" + " * works in OPTIMIZED mode.\n" + " *\n" + " * @type {!Object.}\n" + " */\n" + "$class$.extensionsBinary = {};\n" + "\n", + "class", GetPath(options, desc)); + } } } @@ -2571,7 +2594,7 @@ void Generator::GenerateClassDeserializeBinary(const GeneratorOptions& options, " default:\n"); if (IsExtendable(desc)) { printer->Print( - " jspb.Message.readBinaryExtension(msg, reader, $extobj$,\n" + " jspb.Message.readBinaryExtension(msg, reader, $extobj$Binary,\n" " $class$.prototype.getExtension,\n" " $class$.prototype.setExtension);\n" " break;\n", @@ -2705,8 +2728,8 @@ void Generator::GenerateClassSerializeBinary(const GeneratorOptions& options, if (IsExtendable(desc)) { printer->Print( - " jspb.Message.serializeBinaryExtensions(this, writer, $extobj$,\n" - " $class$.prototype.getExtension);\n", + " jspb.Message.serializeBinaryExtensions(this, writer,\n" + " $extobj$Binary, $class$.prototype.getExtension);\n", "extobj", JSExtensionsObjectName(options, desc->file(), desc), "class", GetPath(options, desc)); } @@ -2874,7 +2897,7 @@ void Generator::GenerateExtension(const GeneratorOptions& options, " /** @type {?function((boolean|undefined),!jspb.Message=): " "!Object} */ (\n" " $toObject$),\n" - " $repeated$", + " $repeated$);\n", "index", SimpleItoa(field->number()), "name", JSObjectFieldName(options, field), "ctor", (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE ? @@ -2886,12 +2909,18 @@ void Generator::GenerateExtension(const GeneratorOptions& options, if (options.binary) { printer->Print( - ",\n" + "\n" + "$extendName$Binary[$index$] = new jspb.ExtensionFieldBinaryInfo(\n" + " $class$.$name$,\n" " $binaryReaderFn$,\n" " $binaryWriterFn$,\n" " $binaryMessageSerializeFn$,\n" - " $binaryMessageDeserializeFn$,\n" - " $isPacked$);\n", + " $binaryMessageDeserializeFn$,\n", + "extendName", JSExtensionsObjectName(options, field->file(), + field->containing_type()), + "index", SimpleItoa(field->number()), + "class", extension_scope, + "name", JSObjectFieldName(options, field), "binaryReaderFn", JSBinaryReaderMethodName(options, field), "binaryWriterFn", JSBinaryWriterMethodName(options, field), "binaryMessageSerializeFn", @@ -2901,10 +2930,11 @@ void Generator::GenerateExtension(const GeneratorOptions& options, "binaryMessageDeserializeFn", (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) ? (SubmessageTypeRef(options, field) + - ".deserializeBinaryFromReader") : "null", + ".deserializeBinaryFromReader") : "null"); + + printer->Print( + " $isPacked$);\n", "isPacked", (field->is_packed() ? "true" : "false")); - } else { - printer->Print(");\n"); } printer->Print(