Comparison and streaming operators for base::Version

chrome/common/chrome_content_client.cc

Index: chrome/common/chrome_content_client.cc
diff --git a/chrome/common/chrome_content_client.cc b/chrome/common/chrome_content_client.cc
index 50064fb5e0a513bb532e02e2bb0c39ccde5a426c..b69a218e3fcb9e0481365cf8d2fad04d808ddbff 100644
--- a/chrome/common/chrome_content_client.cc
+++ b/chrome/common/chrome_content_client.cc
@@ -470,8 +470,9 @@ content::PepperPluginInfo* ChromeContentClient::FindMostRecentPlugin(
         Version version_x(x->version);
         Version version_y(y->version);
         DCHECK(version_x.IsValid() && version_y.IsValid());
-        if (version_x.Equals(version_y))
+        if (version_x == version_y) {

[brettw, 2016/01/08 19:31:16]

There's no need to add {} here: generally we wouldn't do this for single-line
ones.

[Rob Percival, 2016/01/09 23:54:03]

Done. I'm in the habit of putting braces in since I saw a critical iOS TLS bug
(http://arstechnica.com/security/2014/02/extremely-critical-crypto-flaw-in-ios...)
that might have been avoided if braces had been used. Since that's not the
convention for Chromium, I'll refrain in the future.

           return !x->is_debug && y->is_debug;
+        }
         return version_x.IsOlderThan(y->version);

[brettw, 2016/01/08 19:31:16]

Can you update this to:
  return version_x < version_y;
As-is this will parse the Y version string again to do the comparison which is
wasteful since we already did that.

I actually think we should remove IsOlderThan entirely. This falls under the
avoidance of operators rule, and it's only used in a couple of places.

I see only 15 occurrences in the code of this call and many of those aren't
Version objects so don't need to be updated. Of the rest, some are silly and
wasteful like this one and extension_service_unittest.cc. So let's just get rid
of those while we're at it.

[Rob Percival, 2016/01/09 23:54:03]

Done.

       });
   return it != plugins.end() ? *it : nullptr;