Mojo: nuke EnvironmentData

With this change, Mojo applications that link against mojo_environment_chromium
do not need to instantiate mojo::Environment. We rely on AtExitManager for all
finalization of singleton objects. This frees us up to use the familiar
base::Singleton and base::LazyInstance for any such state. Tests can use
ShadowingAtExitManager to clean the environment between test runs.

It becomes a link error to use mojo::Environment if you are not linking against
mojo_environment_standalone. I plan to follow this up with a change that buries
mojo::Environment for the case where you are linking against
mojo_environment_standalone. Ideally, this means no one will ever need to think
about mojo::Environment again.

Review URL: https://codereview.chromium.org/281353005

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

18 files changed, 19 insertions, 179 deletions

diff --git a/mojo/android/javatests/core_test.cc b/mojo/android/javatests/core_test.cc
index 38aa6c4..2c2054d 100644
--- a/mojo/android/javatests/core_test.cc
+++ b/mojo/android/javatests/core_test.cc
@@ -17,7 +17,6 @@
 namespace {
 
 struct TestEnvironment {
-  mojo::Environment environment;
   base::MessageLoopForUI message_loop;
 };
 
diff --git a/mojo/apps/js/test/js_to_cpp_unittest.cc b/mojo/apps/js/test/js_to_cpp_unittest.cc
index 504f59b..67e0957 100644
--- a/mojo/apps/js/test/js_to_cpp_unittest.cc
+++ b/mojo/apps/js/test/js_to_cpp_unittest.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/at_exit.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/message_loop/message_loop.h"
@@ -13,7 +14,6 @@
 #include "mojo/common/common_type_converters.h"
 #include "mojo/common/test/test_utils.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/public/cpp/system/macros.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -414,7 +414,7 @@ class JsToCppTest : public testing::Test {
   }
 
  private:
-  Environment environment;
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop;
   base::RunLoop run_loop_;
 
diff --git a/mojo/common/environment_data.cc b/mojo/common/environment_data.cc
deleted file mode 100644
index b523d27..0000000
--- a/mojo/common/environment_data.cc
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "mojo/common/environment_data.h"
-
-#include "base/stl_util.h"
-
-namespace mojo {
-namespace common {
-
-// static
-EnvironmentData* EnvironmentData::instance_ = NULL;
-
-EnvironmentData::EnvironmentData() {
-  DCHECK(!instance_);
-  instance_ = this;
-}
-
-EnvironmentData::~EnvironmentData() {
-  instance_ = NULL;
-  DataMap data_map;
-  data_map.swap(data_map_);
-  STLDeleteContainerPairSecondPointers(data_map.begin(), data_map.end());
-}
-
-// static
-EnvironmentData* EnvironmentData::GetInstance() {
-  return instance_;
-}
-
-void EnvironmentData::SetData(const void* key, scoped_ptr<Data> data) {
-  Data* old = NULL;
-  {
-    base::AutoLock auto_lock(data_lock_);
-    old = data_map_[key];
-    if (data)
-      data_map_[key] = data.release();
-    else
-      data_map_.erase(key);
-  }
-  delete old;
-}
-
-EnvironmentData::Data* EnvironmentData::GetData(const void* key) {
-  base::AutoLock auto_lock(data_lock_);
-  return data_map_.count(key) > 0 ? data_map_[key] : NULL;
-}
-
-}  // namespace common
-}  // namespace mojo
diff --git a/mojo/common/environment_data.h b/mojo/common/environment_data.h
deleted file mode 100644
index 4440df2..0000000
--- a/mojo/common/environment_data.h
+++ /dev/null
@@ -1,52 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef MOJO_COMMON_ENVIRONMENT_DATA_H_
-#define MOJO_COMMON_ENVIRONMENT_DATA_H_
-
-#include <map>
-
-#include "base/memory/scoped_ptr.h"
-#include "base/synchronization/lock.h"
-#include "mojo/common/mojo_common_export.h"
-
-namespace mojo {
-namespace common {
-
-// EnvironmentData is used to store arbitrary key/value pairs in the
-// environment. The key/value pairs are owned by the Environment and deleted
-// when it is deleted.
-class MOJO_COMMON_EXPORT EnvironmentData {
- public:
-  class MOJO_COMMON_EXPORT Data {
-   public:
-    Data() {}
-    virtual ~Data() {}
-  };
-
-  EnvironmentData();
-  ~EnvironmentData();
-
-  static EnvironmentData* GetInstance();
-
-  void SetData(const void* key, scoped_ptr<Data> data);
-
-  Data* GetData(const void* key);
-
- private:
-  typedef std::map<const void*, Data*> DataMap;
-
-  static EnvironmentData* instance_;
-
-  base::Lock data_lock_;
-
-  DataMap data_map_;
-
-  DISALLOW_COPY_AND_ASSIGN(EnvironmentData);
-};
-
-}  // namespace common
-}  // namespace mojo
-
-#endif  // MOJO_COMMON_ENVIRONMENT_DATA_H_
diff --git a/mojo/common/handle_watcher.cc b/mojo/common/handle_watcher.cc
index 1affa90..4ae86db 100644
--- a/mojo/common/handle_watcher.cc
+++ b/mojo/common/handle_watcher.cc
@@ -9,13 +9,13 @@
 #include "base/atomic_sequence_num.h"
 #include "base/bind.h"
 #include "base/lazy_instance.h"
+#include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
-#include "mojo/common/environment_data.h"
 #include "mojo/common/message_pump_mojo.h"
 #include "mojo/common/message_pump_mojo_handler.h"
 #include "mojo/common/time_helper.h"
@@ -29,8 +29,6 @@ namespace {
 
 const char kWatcherThreadName[] = "handle-watcher-thread";
 
-const char kWatcherThreadManagerKey[] = "watcher-thread-manager";
-
 // TODO(sky): this should be unnecessary once MessageLoop has been refactored.
 MessagePumpMojo* message_pump_mojo = NULL;
 
@@ -176,6 +174,7 @@ class WatcherThreadManager {
   void StopWatching(WatcherID watcher_id);
 
  private:
+  friend struct DefaultSingletonTraits<WatcherThreadManager>;
   WatcherThreadManager();
 
   base::Thread thread_;
@@ -187,29 +186,12 @@ class WatcherThreadManager {
   DISALLOW_COPY_AND_ASSIGN(WatcherThreadManager);
 };
 
-struct WatcherThreadManagerData : EnvironmentData::Data {
-  scoped_ptr<WatcherThreadManager> thread_manager;
-};
-
 WatcherThreadManager::~WatcherThreadManager() {
   thread_.Stop();
 }
 
-static base::LazyInstance<base::Lock> thread_lookup_lock =
-    LAZY_INSTANCE_INITIALIZER;
-
 WatcherThreadManager* WatcherThreadManager::GetInstance() {
-  base::AutoLock auto_lock(thread_lookup_lock.Get());
-  WatcherThreadManagerData* data = static_cast<WatcherThreadManagerData*>(
-      EnvironmentData::GetInstance()->GetData(kWatcherThreadManagerKey));
-  if (!data) {
-    data = new WatcherThreadManagerData;
-    data->thread_manager.reset(new WatcherThreadManager);
-    EnvironmentData::GetInstance()->SetData(
-        kWatcherThreadManagerKey,
-        scoped_ptr<EnvironmentData::Data>(data));
-  }
-  return data->thread_manager.get();
+  return Singleton<WatcherThreadManager>::get();
 }
 
 WatcherID WatcherThreadManager::StartWatching(
diff --git a/mojo/common/handle_watcher_unittest.cc b/mojo/common/handle_watcher_unittest.cc
index 37fde23..39667814 100644
--- a/mojo/common/handle_watcher_unittest.cc
+++ b/mojo/common/handle_watcher_unittest.cc
@@ -6,12 +6,12 @@
 
 #include <string>
 
+#include "base/at_exit.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "base/test/simple_test_tick_clock.h"
 #include "mojo/common/time_helper.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/system/core.h"
 #include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -116,7 +116,7 @@ class HandleWatcherTest : public testing::Test {
   base::SimpleTestTickClock tick_clock_;
 
  private:
-  Environment environment_;
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(HandleWatcherTest);
@@ -318,8 +318,7 @@ TEST(HandleWatcherCleanEnvironmentTest, AbortedOnMessageLoopDestruction) {
   bool was_signaled = false;
   MojoResult result = MOJO_RESULT_OK;
 
-  Environment env;
-
+  base::ShadowingAtExitManager at_exit;
   MessagePipe pipe;
   HandleWatcher watcher;
   {
diff --git a/mojo/environment/environment.cc b/mojo/environment/environment.cc
index ab182a6..468f6a3 100644
--- a/mojo/environment/environment.cc
+++ b/mojo/environment/environment.cc
@@ -4,32 +4,16 @@
 
 #include "mojo/public/cpp/environment/environment.h"
 
-#include "mojo/common/environment_data.h"
-
 namespace mojo {
 
-class Environment::Data {
- public:
-  Data();
-  ~Data();
-
- private:
-  common::EnvironmentData data_;
-
-  DISALLOW_COPY_AND_ASSIGN(Data);
-};
-
-Environment::Data::Data() {
-}
-
-Environment::Data::~Data() {
-}
-
-Environment::Environment() : data_(new Environment::Data) {
+// These methods are intentionally not implemented so that there is a link
+// error if someone uses them in a Chromium-environment.
+#if 0
+Environment::Environment() {
 }
 
 Environment::~Environment() {
-  delete data_;
 }
+#endif
 
 }  // namespace mojo
diff --git a/mojo/examples/pepper_container_app/pepper_container_app.cc b/mojo/examples/pepper_container_app/pepper_container_app.cc
index d2aab41..8fccce6 100644
--- a/mojo/examples/pepper_container_app/pepper_container_app.cc
+++ b/mojo/examples/pepper_container_app/pepper_container_app.cc
@@ -12,7 +12,6 @@
 #include "mojo/examples/pepper_container_app/plugin_module.h"
 #include "mojo/examples/pepper_container_app/type_converters.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/gles2/gles2.h"
 #include "mojo/public/cpp/shell/application.h"
 #include "mojo/public/cpp/system/core.h"
@@ -121,7 +120,6 @@ class PepperContainerApp: public Application,
 
 extern "C" PEPPER_CONTAINER_APP_EXPORT MojoResult CDECL MojoMain(
     MojoHandle shell_handle) {
-  mojo::Environment env;
   mojo::GLES2Initializer gles2;
   base::MessageLoop run_loop;
   mojo::examples::PepperContainerApp app(shell_handle);
diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp
index 3893b6b..0340705 100644
--- a/mojo/mojo.gyp
+++ b/mojo/mojo.gyp
@@ -320,8 +320,6 @@
         'common/channel_init.h',
         'common/common_type_converters.cc',
         'common/common_type_converters.h',
-        'common/environment_data.cc',
-        'common/environment_data.h',
         'common/handle_watcher.cc',
         'common/handle_watcher.h',
         'common/message_pump_mojo.cc',
diff --git a/mojo/public/cpp/environment/environment.h b/mojo/public/cpp/environment/environment.h
index df293ae..f75e2f7 100644
--- a/mojo/public/cpp/environment/environment.h
+++ b/mojo/public/cpp/environment/environment.h
@@ -16,11 +16,6 @@ class Environment {
   ~Environment();
 
  private:
-  class Data;
-
-  // Environment implementation can use this to store state.
-  Data* data_;
-
   MOJO_DISALLOW_COPY_AND_ASSIGN(Environment);
 };
 
diff --git a/mojo/public/cpp/environment/lib/environment.cc b/mojo/public/cpp/environment/lib/environment.cc
index 07adc06..b3148af 100644
--- a/mojo/public/cpp/environment/lib/environment.cc
+++ b/mojo/public/cpp/environment/lib/environment.cc
@@ -9,7 +9,7 @@
 
 namespace mojo {
 
-Environment::Environment() : data_(NULL) {
+Environment::Environment() {
   internal::SetUpCurrentBuffer();
   RunLoop::SetUp();
 }
diff --git a/mojo/service_manager/service_manager_unittest.cc b/mojo/service_manager/service_manager_unittest.cc
index fd763b5..d28ba89 100644
--- a/mojo/service_manager/service_manager_unittest.cc
+++ b/mojo/service_manager/service_manager_unittest.cc
@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/at_exit.h"
 #include "base/message_loop/message_loop.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/shell/application.h"
 #include "mojo/public/interfaces/shell/shell.mojom.h"
 #include "mojo/service_manager/service_loader.h"
@@ -180,7 +180,7 @@ class ServiceManagerTest : public testing::Test {
   }
 
  protected:
-  mojo::Environment env_;
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop_;
   TestContext context_;
   scoped_ptr<TestClientImpl> test_client_;
diff --git a/mojo/services/dbus_echo/dbus_echo_service.cc b/mojo/services/dbus_echo/dbus_echo_service.cc
index f9e476e..1753638 100644
--- a/mojo/services/dbus_echo/dbus_echo_service.cc
+++ b/mojo/services/dbus_echo/dbus_echo_service.cc
@@ -45,7 +45,6 @@ int main(int argc, char** argv) {
                        false,    // Timestamp
                        false);   // Tick count
 
-  mojo::Environment env;
   mojo::embedder::Init();
 
   base::MessageLoopForIO message_loop;
diff --git a/mojo/services/view_manager/view_manager_connection_unittest.cc b/mojo/services/view_manager/view_manager_connection_unittest.cc
index fe507b5e..6dbd675 100644
--- a/mojo/services/view_manager/view_manager_connection_unittest.cc
+++ b/mojo/services/view_manager/view_manager_connection_unittest.cc
@@ -5,6 +5,7 @@
 #include <string>
 #include <vector>
 
+#include "base/at_exit.h"
 #include "base/bind.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
@@ -13,7 +14,6 @@
 #include "mojo/common/common_type_converters.h"
 #include "mojo/geometry/geometry_type_converters.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/shell/connect.h"
 #include "mojo/services/public/cpp/view_manager/util.h"
 #include "mojo/services/public/cpp/view_manager/view_manager_types.h"
@@ -415,6 +415,7 @@ class ViewManagerConnectionTest : public testing::Test {
     view_manager2_.reset();
   }
 
+  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop_;
   shell::ShellTestHelper test_helper_;
 
diff --git a/mojo/shell/android/mojo_main.cc b/mojo/shell/android/mojo_main.cc
index 9d70c8f..25b1478 100644
--- a/mojo/shell/android/mojo_main.cc
+++ b/mojo/shell/android/mojo_main.cc
@@ -13,7 +13,6 @@
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "jni/MojoMain_jni.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/shell/application.h"
 #include "mojo/service_manager/service_loader.h"
 #include "mojo/service_manager/service_manager.h"
@@ -34,10 +33,6 @@ LazyInstance<scoped_ptr<base::MessageLoop> > g_java_message_loop =
 LazyInstance<scoped_ptr<shell::Context> > g_context =
     LAZY_INSTANCE_INITIALIZER;
 
-
-LazyInstance<scoped_ptr<mojo::Environment> > g_env =
-    LAZY_INSTANCE_INITIALIZER;
-
 }  // namspace
 
 static void Init(JNIEnv* env, jclass clazz, jobject context) {
@@ -74,8 +69,6 @@ static void Start(JNIEnv* env, jclass clazz, jobject context, jstring jurl) {
     CommandLine::ForCurrentProcess()->InitFromArgv(argv);
   }
 
-  g_env.Get().reset(new Environment);
-
   base::android::ScopedJavaGlobalRef<jobject> activity;
   activity.Reset(env, context);
 
diff --git a/mojo/shell/context.cc b/mojo/shell/context.cc
index 4435f4c..7ad6c24 100644
--- a/mojo/shell/context.cc
+++ b/mojo/shell/context.cc
@@ -49,7 +49,7 @@ class Setup {
   DISALLOW_COPY_AND_ASSIGN(Setup);
 };
 
-static base::LazyInstance<Setup> setup = LAZY_INSTANCE_INITIALIZER;
+static base::LazyInstance<Setup>::Leaky setup = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
diff --git a/mojo/shell/desktop/mojo_main.cc b/mojo/shell/desktop/mojo_main.cc
index 98dc7a9..9591ae8 100644
--- a/mojo/shell/desktop/mojo_main.cc
+++ b/mojo/shell/desktop/mojo_main.cc
@@ -8,7 +8,6 @@
 #include "base/macros.h"  // TODO(vtl): Remove.
 #include "base/message_loop/message_loop.h"
 #include "mojo/common/message_pump_mojo.h"  // TODO(vtl): Remove.
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/shell/child_process.h"
 #include "mojo/shell/child_process_host.h"  // TODO(vtl): Remove.
 #include "mojo/shell/context.h"
@@ -37,7 +36,6 @@ class TestChildProcessHostDelegate
 
 int main(int argc, char** argv) {
   base::AtExitManager at_exit;
-  mojo::Environment env;
   CommandLine::Init(argc, argv);
   mojo::shell::InitializeLogging();
 
diff --git a/mojo/shell/shell_test_helper.h b/mojo/shell/shell_test_helper.h
index 5d0df6e..76dd62c 100644
--- a/mojo/shell/shell_test_helper.h
+++ b/mojo/shell/shell_test_helper.h
@@ -9,7 +9,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
 #include "base/threading/thread.h"
-#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/interfaces/shell/shell.mojom.h"
 
 namespace base {
@@ -41,8 +40,6 @@ class ShellTestHelper {
   // Invoked once connection has been established.
   void OnShellStarted();
 
-  Environment environment_;
-
   base::Thread shell_thread_;
 
   // If non-null we're in Init() and waiting for connection.