Mojo C++ bindings: switch the existing usage of StructTraits to use the new data view interface.

url/mojo/origin_struct_traits.h

Index: url/mojo/origin_struct_traits.h
diff --git a/url/mojo/origin_struct_traits.h b/url/mojo/origin_struct_traits.h
index 1d28c877db9075b6a68813781c2206326ebe4e6d..cbcd9953944ca09bdc5e8245bc8852c4cb56cd4f 100644
--- a/url/mojo/origin_struct_traits.h
+++ b/url/mojo/origin_struct_traits.h
@@ -25,15 +25,27 @@ struct StructTraits<url::mojom::Origin, url::Origin> {
   static bool unique(const url::Origin& r) {
     return r.unique();
   }
-  static bool Read(url::mojom::Origin::Reader r, url::Origin* out) {
-    *out = r.unique() ? url::Origin()
-                      : url::Origin::UnsafelyCreateOriginWithoutNormalization(
-                            r.scheme(), r.host(), r.port());
+  static bool Read(url::mojom::OriginDataView data, url::Origin* out) {
+    if (data.is_null()) {

[dcheng, 2016/05/11 18:04:22]

Why/when would data be null? Why do we consider this to be a deserialization
success?

[yzshen1, 2016/05/11 18:18:25]

Imagine we have a mojo interface like this:

interface Foo {
  Send(url.mojom.Origin? origin);
};

The sender may send a null url.mojom.Origin. That is legitimate.

Based on the current StructTraits definition, url::Origin (the custom-mapped
type) is never converted to null url.mojom.Origin -- because it doesn't have a
IsNull() function defined (I will update our document to talk about things like
this).

So if the sender is using the custom-mapped url::Origin, it should never send a
null.

But other bindings may be able to send null, for example:
- the standard JS/Java bindings;
- or if the blink bindings use the standard generated
url::mojom::blink::OriginPtr.


You may wonder: what if the interface requires to a non-null Origin:
interface Foo {
  Send(url.mojom.Origin origin);  // non-nullable origin
};
In this case, a null value should be rejected, how can we return true here?

That is because a null |origin| will be rejected directly at the validation
stage. It won't reach this Read() method at all. So whenever we see a null in
this method, it is a legitimate input.


The fact that it takes so long to explain this behavior makes me think that it
is probably easier to understand if we take the approach that Ken and I
discussed in the previous replies of this code review.

WDYT? Thanks!

+      *out = url::Origin();
+      return true;
+    }
+
+    if (data.unique()) {
+      *out = url::Origin();
+    } else {
+      base::StringPiece scheme, host;
+      if (!data.ReadScheme(&scheme) || !data.ReadHost(&host))
+        return false;
+
+      *out = url::Origin::UnsafelyCreateOriginWithoutNormalization(scheme, host,
+                                                                   data.port());
+    }
 
     // If a unique origin was created, but the unique flag wasn't set, then
     // the values provided to 'UnsafelyCreateOriginWithoutNormalization' were
     // invalid.
-    if (!r.unique() && out->unique())
+    if (!data.unique() && out->unique())
       return false;
 
     return true;