JavaScript maps: move binary callbacks out of constructor.

This change will help us separate binary support into
separate files, because we only refer to binary serialization
functions in the actual binary serialization paths.

js/map.js

@@ -44,65 +44,23 @@ goog.forwardDeclare('jspb.BinaryWriter');
  * on ES6 itself.
  *
  * This constructor should only be called from generated message code. It is not
- * intended for general use by library consumers. The callback function
- * arguments are references to methods in `BinaryReader` and `BinaryWriter`, as
- * well as constructors and reader/writer methods in submessage types if
- * appropriate, that are used for binary serialization and parsing.
+ * intended for general use by library consumers.
  *
  * @template K, V
  *
  * @param {!Array<!Array<!Object>>} arr
  *
- * @param {function(this:jspb.BinaryWriter,number,K)=} opt_keyWriterFn
- *     The method on BinaryWriter that writes type K to the stream.
- *
- * @param {function(this:jspb.BinaryReader):K=} opt_keyReaderFn
- *     The method on BinaryReader that reads type K from the stream.
- *
- * @param {function(this:jspb.BinaryWriter,number,V)|
- *         function(this:jspb.BinaryReader,V,?)=} opt_valueWriterFn
- *     The method on BinaryWriter that writes type V to the stream.  May be
- *     writeMessage, in which case the second callback arg form is used.
- *
- * @param {function(this:jspb.BinaryReader):V|
- *         function(this:jspb.BinaryReader,V,
- *                  function(V,!jspb.BinaryReader))=} opt_valueReaderFn
- *    The method on BinaryReader that reads type V from the stream. May be
- *    readMessage, in which case the second callback arg form is used.
- *
  * @param {?function(new:V)|function(new:V,?)=} opt_valueCtor
  *    The constructor for type V, if type V is a message type.
  *
- * @param {?function(V,!jspb.BinaryWriter)=} opt_valueWriterCallback
- *    The BinaryWriter serialization callback for type V, if V is a message
- *    type.
- *
- * @param {?function(V,!jspb.BinaryReader)=} opt_valueReaderCallback
- *    The BinaryReader parsing callback for type V, if V is a message type.
- *
  * @constructor
  * @struct
  */
-jspb.Map = function(
-    arr, opt_keyWriterFn, opt_keyReaderFn, opt_valueWriterFn, opt_valueReaderFn,
-    opt_valueCtor, opt_valueWriterCallback, opt_valueReaderCallback) {
-
+jspb.Map = function(arr, opt_valueCtor) {
   /** @const @private */
   this.arr_ = arr;
   /** @const @private */
-  this.keyWriterFn_ = opt_keyWriterFn;
-  /** @const @private */
-  this.keyReaderFn_ = opt_keyReaderFn;
-  /** @const @private */
-  this.valueWriterFn_ = opt_valueWriterFn;
-  /** @const @private */
-  this.valueReaderFn_ = opt_valueReaderFn;
-  /** @const @private */
   this.valueCtor_ = opt_valueCtor;
-  /** @const @private */
-  this.valueWriterCallback_ = opt_valueWriterCallback;
-  /** @const @private */
-  this.valueReaderCallback_ = opt_valueReaderCallback;
 
   /** @type {!Object<string, !jspb.Map.Entry_<K,V>>} @private */
   this.map_ = {};
@@ -385,19 +343,29 @@ jspb.Map.prototype.has = function(key) {
  * number.
  * @param {number} fieldNumber
  * @param {!jspb.BinaryWriter} writer
+ * @param {function(this:jspb.BinaryWriter,number,K)=} keyWriterFn
+ *     The method on BinaryWriter that writes type K to the stream.
+ * @param {function(this:jspb.BinaryWriter,number,V)|
+ *         function(this:jspb.BinaryReader,V,?)=} valueWriterFn
+ *     The method on BinaryWriter that writes type V to the stream.  May be
+ *     writeMessage, in which case the second callback arg form is used.
+ * @param {?function(V,!jspb.BinaryWriter)=} opt_valueWriterCallback
+ *    The BinaryWriter serialization callback for type V, if V is a message
+ *    type.
  */
-jspb.Map.prototype.serializeBinary = function(fieldNumber, writer) {
+jspb.Map.prototype.serializeBinary = function(
+    fieldNumber, writer, keyWriterFn, valueWriterFn, opt_valueWriterCallback) {
   var strKeys = this.stringKeys_();
   strKeys.sort();
   for (var i = 0; i < strKeys.length; i++) {
     var entry = this.map_[strKeys[i]];
     writer.beginSubMessage(fieldNumber);
-    this.keyWriterFn_.call(writer, 1, entry.key);
+    keyWriterFn.call(writer, 1, entry.key);
     if (this.valueCtor_) {
-      this.valueWriterFn_.call(writer, 2, this.wrapEntry_(entry),
-                               this.valueWriterCallback_);
+      valueWriterFn.call(writer, 2, this.wrapEntry_(entry),
+                         opt_valueWriterCallback);
     } else {
-      this.valueWriterFn_.call(writer, 2, entry.value);
+      valueWriterFn_.call(writer, 2, entry.value);
     }
     writer.endSubMessage();
   }
@@ -410,8 +378,21 @@ jspb.Map.prototype.serializeBinary = function(fieldNumber, writer) {
  * when a key/value pair submessage is encountered.
  * @param {!jspb.Map} map
  * @param {!jspb.BinaryReader} reader
+ * @param {function(this:jspb.BinaryReader):K=} keyReaderFn
+ *     The method on BinaryReader that reads type K from the stream.
+ *
+ * @param {function(this:jspb.BinaryReader):V|
+ *         function(this:jspb.BinaryReader,V,
+ *                  function(V,!jspb.BinaryReader))=} valueReaderFn
+ *    The method on BinaryReader that reads type V from the stream. May be
+ *    readMessage, in which case the second callback arg form is used.
+ *
+ * @param {?function(V,!jspb.BinaryReader)=} opt_valueReaderCallback
+ *    The BinaryReader parsing callback for type V, if V is a message type.
+ *
  */
-jspb.Map.deserializeBinary = function(map, reader) {
+jspb.Map.deserializeBinary = function(map, reader, keyReaderFn, valueReaderFn,
+                                      opt_valueReaderCallback) {
   var key = undefined;
   var value = undefined;
 
@@ -422,14 +403,14 @@ jspb.Map.deserializeBinary = function(map, reader) {
     var field = reader.getFieldNumber();
     if (field == 1) {
       // Key.
-      key = map.keyReaderFn_.call(reader);
+      key = keyReaderFn.call(reader);
     } else if (field == 2) {
       // Value.
       if (map.valueCtor_) {
         value = new map.valueCtor_();
-        map.valueReaderFn_.call(reader, value, map.valueReaderCallback_);
+        valueReaderFn.call(reader, value, opt_valueReaderCallback);
       } else {
-        value = map.valueReaderFn_.call(reader);
+        value = valueReaderFn.call(reader);
       }
     }
   }

js/message.js

@@ -747,29 +747,16 @@ jspb.Message.getFieldProto3 = function(msg, fieldNumber, defaultValue) {
  * of serialization/parsing callbacks (which are required by the map at
  * construction time, and the map may be constructed here).
  *
- * The below callbacks are used to allow the map to serialize and parse its
- * binary wire format data. Their purposes are described in more detail in
- * `jspb.Map`'s constructor documentation.
- *
  * @template K, V
  * @param {!jspb.Message} msg
  * @param {number} fieldNumber
  * @param {boolean|undefined} noLazyCreate
  * @param {?=} opt_valueCtor
- * @param {function(number,K)=} opt_keyWriterFn
- * @param {function():K=} opt_keyReaderFn
- * @param {function(number,V)|function(number,V,?)|
- *         function(number,V,?,?,?,?)=} opt_valueWriterFn
- * @param {function():V|
- *         function(V,function(?,?))=} opt_valueReaderFn
- * @param {function(?,?)|function(?,?,?,?,?)=} opt_valueWriterCallback
- * @param {function(?,?)=} opt_valueReaderCallback
  * @return {!jspb.Map<K, V>|undefined}
  * @protected
  */
 jspb.Message.getMapField = function(msg, fieldNumber, noLazyCreate,
-    opt_valueCtor, opt_keyWriterFn, opt_keyReaderFn, opt_valueWriterFn,
-    opt_valueReaderFn, opt_valueWriterCallback, opt_valueReaderCallback) {
+    opt_valueCtor) {
   if (!msg.wrappers_) {
     msg.wrappers_ = {};
   }
@@ -787,10 +774,7 @@ jspb.Message.getMapField = function(msg, fieldNumber, noLazyCreate,
     }
     return msg.wrappers_[fieldNumber] =
         new jspb.Map(
-            /** @type {!Array<!Array<!Object>>} */ (arr),
-            opt_keyWriterFn, opt_keyReaderFn, opt_valueWriterFn,
-            opt_valueReaderFn, opt_valueCtor, opt_valueWriterCallback,
-            opt_valueReaderCallback);
+            /** @type {!Array<!Array<!Object>>} */ (arr), opt_valueCtor);
   }
 };
 

src/google/protobuf/compiler/js/js_generator.cc

@@ -2253,25 +2253,6 @@ void Generator::GenerateClassField(const GeneratorOptions& options,
           "      null");
     }
 
-    if (options.binary) {
-      printer->Print(",\n"
-          "      $keyWriterFn$,\n"
-          "      $keyReaderFn$,\n"
-          "      $valueWriterFn$,\n"
-          "      $valueReaderFn$",
-          "keyWriterFn", JSBinaryWriterMethodName(options, key_field),
-          "keyReaderFn", JSBinaryReaderMethodName(options, key_field),
-          "valueWriterFn", JSBinaryWriterMethodName(options, value_field),
-          "valueReaderFn", JSBinaryReaderMethodName(options, value_field));
-
-      if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
-        printer->Print(",\n"
-            "      $messageType$.serializeBinaryToWriter,\n"
-            "      $messageType$.deserializeBinaryFromReader",
-            "messageType", GetPath(options, value_field->message_type()));
-      }
-    }
-
     printer->Print(
         "));\n");
 
@@ -2620,10 +2601,25 @@ void Generator::GenerateClassDeserializeBinaryField(
                  "num", SimpleItoa(field->number()));
 
   if (field->is_map()) {
+    const FieldDescriptor* key_field = MapFieldKey(field);
+    const FieldDescriptor* value_field = MapFieldValue(field);
     printer->Print(
         "      var value = msg.get$name$();\n"
-        "      reader.readMessage(value, jspb.Map.deserializeBinary);\n",
+        "      reader.readMessage(value, function(message, reader) {\n",
         "name", JSGetterName(options, field));
+
+    printer->Print("        jspb.Map.deserializeBinary(message, reader, "
+                   "$keyReaderFn$, $valueReaderFn$",
+          "keyReaderFn", JSBinaryReaderMethodName(options, key_field),
+          "valueReaderFn", JSBinaryReaderMethodName(options, value_field));
+
+    if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
+      printer->Print(", $messageType$.deserializeBinaryFromReader",
+          "messageType", GetPath(options, value_field->message_type()));
+    }
+
+    printer->Print(");\n");
+    printer->Print("         });\n");
   } else {
     if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
       printer->Print(
@@ -2782,9 +2778,21 @@ void Generator::GenerateClassSerializeBinaryField(
 
   // Write the field on the wire.
   if (field->is_map()) {
+    const FieldDescriptor* key_field = MapFieldKey(field);
+    const FieldDescriptor* value_field = MapFieldValue(field);
     printer->Print(
-        "    f.serializeBinary($index$, writer);\n",
-        "index", SimpleItoa(field->number()));
+        "    f.serializeBinary($index$, writer, "
+                              "$keyWriterFn$, $valueWriterFn$",
+        "index", SimpleItoa(field->number()),
+        "keyWriterFn", JSBinaryWriterMethodName(options, key_field),
+        "valueWriterFn", JSBinaryWriterMethodName(options, value_field));
+
+    if (value_field->type() == FieldDescriptor::TYPE_MESSAGE) {
+      printer->Print(", $messageType$.serializeBinaryToWriter",
+          "messageType", GetPath(options, value_field->message_type()));
+    }
+
+    printer->Print(");\n");
   } else {
     printer->Print(
         "    writer.write$method$(\n"