Run RemoveArcData after a user has opted out

chrome/browser/chromeos/arc/arc_data_manager.cc

Index: chrome/browser/chromeos/arc/arc_data_manager.cc
diff --git a/chrome/browser/chromeos/arc/arc_data_manager.cc b/chrome/browser/chromeos/arc/arc_data_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a8a98dae96944749aad232054591179e184a409b
--- /dev/null
+++ b/chrome/browser/chromeos/arc/arc_data_manager.cc
@@ -0,0 +1,66 @@
+// Copyright 2016 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 "chrome/browser/chromeos/arc/arc_data_manager.h"
+
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/session_manager_client.h"
+#include "components/prefs/pref_service.h"
+#include "components/user_manager/user.h"
+
+namespace arc {
+
+namespace {
+
+// Weak Pointer.  This class is owned by ChromeBrowserMainPartsChromeos.
+ArcDataManager* g_arc_data_manager = nullptr;
+
+}  // namespace
+
+ArcDataManager::ArcDataManager() {
+  DCHECK(!g_arc_data_manager);
+  g_arc_data_manager = this;
+  ArcBridgeService::Get()->AddObserver(this);
+}
+
+ArcDataManager::~ArcDataManager() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(g_arc_data_manager == this);
+  ArcBridgeService::Get()->RemoveObserver(this);
+  g_arc_data_manager = nullptr;
+}
+
+// static
+ArcDataManager* ArcDataManager::Get() {
+  DCHECK(g_arc_data_manager);
+  DCHECK(g_arc_data_manager->thread_checker_.CalledOnValidThread());
+  return g_arc_data_manager;
+}
+
+void ArcDataManager::OnStateChanged(ArcBridgeService::State state) {
+  DCHECK(g_arc_data_manager->thread_checker_.CalledOnValidThread());
+  if (state != ArcBridgeService::State::STOPPED) {
+    return;
+  }
+  const user_manager::User* const primary_user =
+      user_manager::UserManager::Get()->GetPrimaryUser();

[stevenjb, 2016/05/13 17:13:55]

nit: The extra const here and in line 51 is a bit confusing to read and of
little benefit. This pattern is uncommon in chrome and I would avoid it. (i.e.
just use 'const user_manager::User* primary_user')

[dspaid, 2016/05/15 23:58:51]

Probably a copy/paste bug on my side.

+  Profile* const profile =
+      chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
+  ClearIfDisabled(profile);
+}
+
+void ArcDataManager::ClearIfDisabled(Profile* profile) {
+  DCHECK(g_arc_data_manager->thread_checker_.CalledOnValidThread());
+  DCHECK(ArcBridgeService::Get()->state() == ArcBridgeService::State::STOPPED);

[stevenjb, 2016/05/13 17:13:55]

Is the calling code logically protected against this occurring? If there is any
possibility of an edge case triggering this DCHECk then we should handle this
instead (e.g. with LOG(ERROR) + return).

[dspaid, 2016/05/15 23:58:51]

Done.

+  if (!profile->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {

[stevenjb, 2016/05/13 17:13:54]

nit: Invert and early exit

[dspaid, 2016/05/15 23:58:51]

Done.

+    chromeos::SessionManagerClient* session_manager_client =
+        chromeos::DBusThreadManager::Get()->GetSessionManagerClient();
+    session_manager_client->RemoveArcData();
+  }
+}
+
+}  // namespace arc