Revert 272472 "Mojo: nuke EnvironmentData"

Caused layout tests (when run with check-sys-deps) to hang on Windows
with a stuck content_shell process. See Issue 376929.

BUG=376929

> 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

TBR=darin@chromium.org

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

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

content/app/content_main_runner.cc

@@ -751,6 +751,10 @@ class ContentMainRunnerImpl : public ContentMainRunner {
       delegate_->ProcessExiting(process_type);
     }
 
+#if !defined(OS_IOS)
+    ShutdownMojo();
+#endif
+
 #if defined(OS_WIN)
 #ifdef _CRTDBG_MAP_ALLOC
     _CrtDumpMemoryLeaks();

content/app/mojo/mojo_init.cc

@@ -6,13 +6,27 @@
 
 #include "base/logging.h"
 #include "mojo/embedder/embedder.h"
+#include "mojo/public/cpp/environment/environment.h"
 #include "mojo/service_manager/service_manager.h"
 
 namespace content {
 
+namespace {
+
+mojo::Environment* environment = NULL;
+
+}  // namespace
+
 void InitializeMojo() {
+  DCHECK(!environment);
+  environment = new mojo::Environment;
   mojo::embedder::Init();
   mojo::ServiceManager::GetInstance();
 }
 
+void ShutdownMojo() {
+  delete environment;
+  environment = NULL;
+}
+
 }  // namespace content

content/app/mojo/mojo_init.h

@@ -12,6 +12,8 @@ namespace content {
 // Perform any necessary Mojo initialization.
 CONTENT_EXPORT void InitializeMojo();
 
+CONTENT_EXPORT void ShutdownMojo();
+
 }  // namespace content
 
 #endif  // CONTENT_COMMON_MOJO_MOJO_INIT_H_

content/test/content_test_launcher.cc

@@ -76,6 +76,14 @@ class ContentBrowserTestSuite : public ContentTestSuiteBase {
     ContentTestSuiteBase::Initialize();
   }
 
+  virtual void Shutdown() OVERRIDE {
+    ContentTestSuiteBase::Shutdown();
+
+#if defined(OS_ANDROID)
+    ShutdownMojo();
+#endif
+  }
+
 #if defined(OS_ANDROID)
   scoped_ptr<ShellContentClient> content_client_;
   scoped_ptr<ShellContentBrowserClient> browser_content_client_;

mojo/android/javatests/core_test.cc

@@ -17,6 +17,7 @@
 namespace {
 
 struct TestEnvironment {
+  mojo::Environment environment;
   base::MessageLoopForUI message_loop;
 };
 

mojo/apps/js/test/js_to_cpp_unittest.cc

@@ -2,7 +2,6 @@
 // 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"
@@ -14,6 +13,7 @@
 #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:
-  base::ShadowingAtExitManager at_exit_;
+  Environment environment;
   base::MessageLoop loop;
   base::RunLoop run_loop_;
 

mojo/common/environment_data.cc

+// 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

mojo/common/environment_data.h

+// 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_

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,6 +29,8 @@ 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;
 
@@ -174,7 +176,6 @@ class WatcherThreadManager {
   void StopWatching(WatcherID watcher_id);
 
  private:
-  friend struct DefaultSingletonTraits<WatcherThreadManager>;
   WatcherThreadManager();
 
   base::Thread thread_;
@@ -186,12 +187,29 @@ 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() {
-  return Singleton<WatcherThreadManager>::get();
+  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();
 }
 
 WatcherID WatcherThreadManager::StartWatching(

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:
-  base::ShadowingAtExitManager at_exit_;
+  Environment environment_;
   base::MessageLoop message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(HandleWatcherTest);
@@ -318,7 +318,8 @@ TEST(HandleWatcherCleanEnvironmentTest, AbortedOnMessageLoopDestruction) {
   bool was_signaled = false;
   MojoResult result = MOJO_RESULT_OK;
 
-  base::ShadowingAtExitManager at_exit;
+  Environment env;
+
   MessagePipe pipe;
   HandleWatcher watcher;
   {

mojo/environment/environment.cc

@@ -4,16 +4,32 @@
 
 #include "mojo/public/cpp/environment/environment.h"
 
+#include "mojo/common/environment_data.h"
+
 namespace mojo {
 
-// 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() {
+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) {
 }
 
 Environment::~Environment() {
+  delete data_;
 }
-#endif
 
 }  // namespace mojo

mojo/examples/pepper_container_app/pepper_container_app.cc

@@ -12,6 +12,7 @@
 #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"
@@ -120,6 +121,7 @@ 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);

mojo/mojo.gyp

@@ -320,6 +320,8 @@
         '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',

mojo/public/cpp/environment/environment.h

@@ -16,6 +16,11 @@ class Environment {
   ~Environment();
 
  private:
+  class Data;
+
+  // Environment implementation can use this to store state.
+  Data* data_;
+
   MOJO_DISALLOW_COPY_AND_ASSIGN(Environment);
 };
 

mojo/public/cpp/environment/lib/environment.cc

@@ -9,7 +9,7 @@
 
 namespace mojo {
 
-Environment::Environment() {
+Environment::Environment() : data_(NULL) {
   internal::SetUpCurrentBuffer();
   RunLoop::SetUp();
 }

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:
-  base::ShadowingAtExitManager at_exit_;
+  mojo::Environment env_;
   base::MessageLoop loop_;
   TestContext context_;
   scoped_ptr<TestClientImpl> test_client_;

mojo/services/dbus_echo/dbus_echo_service.cc

@@ -45,6 +45,7 @@ int main(int argc, char** argv) {
                        false,    // Timestamp
                        false);   // Tick count
 
+  mojo::Environment env;
   mojo::embedder::Init();
 
   base::MessageLoopForIO message_loop;

mojo/services/view_manager/view_manager_connection_unittest.cc

@@ -5,7 +5,6 @@
 #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"
@@ -14,6 +13,7 @@
 #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,7 +415,6 @@ class ViewManagerConnectionTest : public testing::Test {
     view_manager2_.reset();
   }
 
-  base::ShadowingAtExitManager at_exit_;
   base::MessageLoop loop_;
   shell::ShellTestHelper test_helper_;
 

mojo/shell/android/mojo_main.cc

@@ -13,6 +13,7 @@
 #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"
@@ -33,6 +34,10 @@ 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) {
@@ -69,6 +74,8 @@ 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);
 

mojo/shell/context.cc

@@ -49,7 +49,7 @@ class Setup {
   DISALLOW_COPY_AND_ASSIGN(Setup);
 };
 
-static base::LazyInstance<Setup>::Leaky setup = LAZY_INSTANCE_INITIALIZER;
+static base::LazyInstance<Setup> setup = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 

mojo/shell/desktop/mojo_main.cc

@@ -8,6 +8,7 @@
 #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"
@@ -36,6 +37,7 @@ class TestChildProcessHostDelegate
 
 int main(int argc, char** argv) {
   base::AtExitManager at_exit;
+  mojo::Environment env;
   CommandLine::Init(argc, argv);
   mojo::shell::InitializeLogging();
 

mojo/shell/shell_test_helper.h

@@ -9,6 +9,7 @@
 #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 {
@@ -40,6 +41,8 @@ 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.