The features request broke backward compatibility for code currently in the wild.

This CL fixes that as follows.

If there is no 'o3d_features' argument passed to the plugin it assumes the code
using the plugin is old and turns on the features that broke backward compatibility.
Otherwise, the utility libraries always pass in a o3d_features argument from this
point on so it will work the new way (ie, certain features must be requested).
Also, the requested version is passed in as well. This way we can adjust the API
by version number later.

Unfortunately, as it is, neither the o3d_features argument is required nor
is the version number which means if someone is not using the o3djs libraries
it would be possible for them to make something that we would break later.

We could maybe fix that by requiring a version number and failing if one is
not passed in  although old code will still not give us a version number so I'm
really not sure how to handle that case.

Review URL: http://codereview.chromium.org/118050

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@17994 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 40 insertions, 9 deletions

diff --git a/o3d/core/cross/features.cc b/o3d/core/cross/features.cc
index ce7d41d..75349cc 100644
--- a/o3d/core/cross/features.cc
+++ b/o3d/core/cross/features.cc
@@ -45,14 +45,23 @@ const InterfaceId Features::kInterfaceId =
 
 Features::Features(ServiceLocator* service_locator)
     : service_(service_locator, this),
-      floating_point_textures_(false),
-      large_geometry_(false),
+      floating_point_textures_(true),
+      large_geometry_(true),
       windowless_(false),
       not_anti_aliased_(false),
       init_status_(Renderer::SUCCESS) {
+  // NOTE: For backward compatibility floating_point_textures and
+  //     large_geometry default to true.  o3djs.util.makeClients before 0.1.35.0
+  //     does not set the o3d_features plugin parameters and therefore
+  //     Features::Init is not called.  o3djs,util.makeClients after and
+  //     including 0.1.35.0 do set o3d_features and therefore Init is called
+  //     which sets those to false to start.
 }
 
 void Features::Init(const String& requested_features) {
+  large_geometry_ = false;
+  floating_point_textures_ = false;
+
   std::vector<std::string> features;
   SplitString(requested_features, ',', &features);
   for (size_t jj = 0; jj < features.size(); ++jj) {
diff --git a/o3d/core/cross/features_test.cc b/o3d/core/cross/features_test.cc
index 6dc43d5..bd2e339 100644
--- a/o3d/core/cross/features_test.cc
+++ b/o3d/core/cross/features_test.cc
@@ -63,6 +63,28 @@ class FeaturesTest : public testing::Test {
 TEST_F(FeaturesTest, Basic) {
   Features* features = new Features(service_locator());
 
+  // Check that the features start off correctly.
+  //
+  // NOTE: For backward compatibility floating_point_textures and
+  //     large_geometry default to true.  o3djs.util.makeClients before 0.1.35.0
+  //     does not set the o3d_features plugin parameters and therefore
+  //     Features::Init is not called.  o3djs,util.makeClients after and
+  //     including 0.1.35.0 do set o3d_features and therefore Init is called
+  //     which sets those to false to start.
+  EXPECT_TRUE(features->floating_point_textures());
+  EXPECT_TRUE(features->large_geometry());
+  EXPECT_FALSE(features->windowless());
+  EXPECT_FALSE(features->not_anti_aliased());
+  EXPECT_EQ(features->init_status(), Renderer::SUCCESS);
+
+  delete features;
+}
+
+TEST_F(FeaturesTest, Empty) {
+  Features* features = new Features(service_locator());
+
+  features->Init("");
+
   // Check that the features start off as false.
   EXPECT_FALSE(features->floating_point_textures());
   EXPECT_FALSE(features->large_geometry());
diff --git a/o3d/samples/o3djs/util.js b/o3d/samples/o3djs/util.js
index b898a24..e6eaae5 100644
--- a/o3d/samples/o3djs/util.js
+++ b/o3d/samples/o3djs/util.js
@@ -60,7 +60,7 @@ o3djs.util.PLUGIN_NAME = 'O3D Plugin';
  * utility libraries.
  * @type {string}
  */
-o3djs.util.REQUIRED_VERSION = '0.1.34.4';
+o3djs.util.REQUIRED_VERSION = '0.1.35.0';
 
 /**
  * A URL at which to download the client.
@@ -713,10 +713,13 @@ o3djs.util.getScriptTagText_ = function() {
  *    available.
  */
 o3djs.util.createClient = function(element, opt_features, opt_requestVersion) {
-  if (opt_requestVersion &&
-      !o3djs.util.requiredVersionAvailable(opt_requestVersion)) {
+  opt_features = opt_features || '';
+  opt_requestVersion = opt_requestVersion || o3djs.util.REQUIRED_VERSION;
+  if (!o3djs.util.requiredVersionAvailable(opt_requestVersion)) {
     return null;
   }
+  opt_features += (opt_features ? ',' : '') + 'APIVersion=' +
+                  opt_requestVersion;
   var objElem;
   // TODO: Use opt_requiredVersion to set a version so the plugin
   //    can make sure it offers that version of the API.
@@ -728,7 +731,6 @@ o3djs.util.createClient = function(element, opt_features, opt_requestVersion) {
           'WIDTH="100%" HEIGHT="100%"' +
           'CLASSID="CLSID:9666A772-407E-4F90-BC37-982E8160EB2D">' +
             '<PARAM name="o3d_features" value="' + opt_features + '"/>' +
-            '<PARAM name="version" value="' + opt_requestVersion + '"/>' +
         '</OBJECT>';
     objElem = element.childNodes[0];
   } else {
@@ -737,7 +739,6 @@ o3djs.util.createClient = function(element, opt_features, opt_requestVersion) {
     objElem.style.width = '100%';
     objElem.style.height = '100%';
     objElem.setAttribute('o3d_features', opt_features);
-    objElem.version = opt_requestVersion;
     element.appendChild(objElem);
   }
 
@@ -804,8 +805,7 @@ o3djs.util.makeClients = function(callback,
   var tag = opt_tag || 'div';
   var id = opt_id || '^o3d';
   opt_failureCallback = opt_failureCallback || o3djs.util.informPluginFailure;
-  opt_requiredVersion = opt_requiredVersion ||
-      o3djs.util.REQUIRED_VERSION;
+  opt_requiredVersion = opt_requiredVersion || o3djs.util.REQUIRED_VERSION;
   if (!o3djs.util.requiredVersionAvailable(opt_requiredVersion)) {
     opt_failureCallback(o3djs.util.rendererInitStatus.NO_PLUGIN, '', id, tag);
   } else {