Blimp Settings framework on the c++ side

blimp/client/core/settings/settings.cc

Index: blimp/client/core/settings/settings.cc
diff --git a/blimp/client/core/settings/settings.cc b/blimp/client/core/settings/settings.cc
new file mode 100644
index 0000000000000000000000000000000000000000..08c2ffe4e923a1b1058e2e226dc3ea99be45c3c1
--- /dev/null
+++ b/blimp/client/core/settings/settings.cc
@@ -0,0 +1,72 @@
+// 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 "blimp/client/core/settings/settings.h"
+
+#include "base/command_line.h"
+#include "blimp/client/core/blimp_client_switches.h"
+#include "blimp/client/core/settings/settings_observer.h"
+#include "blimp/client/core/settings/settings_prefs.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+
+namespace blimp {
+namespace client {
+
+Settings::Settings(PrefService* local_state)
+    : local_state_(local_state),
+      record_whole_document_(false),
+      show_network_stats_(false) {}
+
+Settings::~Settings() {}

[David Trainor- moved to gerrit, 2016/09/27 04:10:06]

= default

[Menglin, 2016/09/30 23:48:57]

Done.

+
+void Settings::AddObserver(std::unique_ptr<SettingsObserver> observer) {
+  observers_.push_back(std::move(observer));
+}
+
+void Settings::SetShowNetworkStats(bool enable) {
+  show_network_stats_ = enable;
+  for (const auto& observer : observers_) {

[David Trainor- moved to gerrit, 2016/09/27 04:10:06]

Use the observer macro?

[Menglin, 2016/09/30 23:48:57]

Done.

+    observer->OnShowNetworkStatsChanged(show_network_stats_);
+  }
+}
+
+void Settings::SetEnableBlimpMode(bool enable) {
+  blimp_enabled_ = enable;
+  local_state_->SetBoolean(prefs::kBlimpEnabled, enable);

[David Trainor- moved to gerrit, 2016/09/27 04:10:06]

Notify the enable blimp mode state changed first?

[Menglin, 2016/09/30 23:48:56]

Done.

+  for (const auto& observer : observers_) {
+    observer->OnRestartRequired();
+  }
+}
+
+void Settings::SetRecordWholeDocument(bool enable) {
+  if (record_whole_document_ == enable) return;

[David Trainor- moved to gerrit, 2016/09/27 04:10:06]

Put this on the others as well?

[Menglin, 2016/09/30 23:48:56]

Done.

+
+  record_whole_document_ = enable;
+  local_state_->SetBoolean(prefs::kRecordWholeDocument, record_whole_document_);
+  for (const auto& observer : observers_) {
+    observer->OnRecordWholeDocumentChanged(record_whole_document_);
+  }

[David Trainor- moved to gerrit, 2016/09/27 04:10:06]

Also requires restart

[Menglin, 2016/09/30 23:48:57]

Done.

[Menglin, 2016/10/24 22:44:58]

I'm removing OnRestartRequired() for this and using OnRecordWholeDocumentChanged

+}
+
+void Settings::InitializeFromCommandLineAndPref() {

[David Trainor- moved to gerrit, 2016/09/27 04:10:06]

Let's be consistent with how we do this (call set or just set the variable).

[Menglin, 2016/09/30 23:48:57]

ok. After the change, InitializeFromCommandLineAndPref() only takes care of
updating private members and saving to the persistent storage.

sending the settings to the engine is moved to SettingsFeature::PushSettings()

+  const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
+
+  blimp_enabled_ = cmd_line->HasSwitch(switches::kEnableBlimp) ||
+      local_state_->GetBoolean(prefs::kBlimpEnabled);
+
+  bool record_whole_document_enabled =
+      cmd_line->HasSwitch(switches::kDownloadWholeDocument) ||
+      local_state_->GetBoolean(prefs::kRecordWholeDocument);
+  SetRecordWholeDocument(record_whole_document_enabled);
+}
+
+// static
+void Settings::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterBooleanPref(prefs::kBlimpEnabled, false);
+  registry->RegisterBooleanPref(prefs::kRecordWholeDocument, false);
+}
+
+}  // namespace client
+}  // namespace blimp