commit ccd0957b359136aca2d4d5c7728535d8a2e33c51
author Gilad Arnold <garnold@chromium.org> Mon Oct 27 13:37:50 2014 -0700
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Fri Oct 31 04:29:15 2014 +0000
tree 48bfa3104b6bd0c8730710bcf87869e81713787a
parent 560ae1da0e62f7897699f63631452f0c4144d413

update_engine: Move P2P enabled pref handling into P2PManager.

This makes the P2PManager own the manipulation of the P2P enabled
setting, which positions us better toward moving the decision about
whether P2P is enabled from P2PManager and into the UpdateManager. Also
increases encapsulation and makes it a bit more resilient.

BUG=chromium:425233
TEST=Unit tests.

Change-Id: Ic91872df84920ae80c5ef973aee99cc46cac264c
Reviewed-on: https://chromium-review.googlesource.com/225681
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Gaurav Shah <gauravsh@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>

p2p_manager.cc

diff --git a/p2p_manager.cc b/p2p_manager.cc
index b1383e2..9f0db53 100644
--- a/p2p_manager.cc
+++ b/p2p_manager.cc
@@ -118,6 +118,7 @@
                               bool *out_result);
   virtual bool FileMakeVisible(const string& file_id);
   virtual int CountSharedFiles();
+  bool SetP2PEnabledPref(bool enabled) override;
 
  private:
   // Enumeration for specifying visibility.
@@ -161,6 +162,9 @@
   // The string ".tmp".
   static const char kTmpExtension[];
 
+  // Whether P2P service may be running; initially, we assume it may be.
+  bool may_be_running_ = true;
+
   DISALLOW_COPY_AND_ASSIGN(P2PManagerImpl);
 };
 
@@ -236,6 +240,8 @@
   GError *error = nullptr;
   gint exit_status = 0;
 
+  may_be_running_ = true;  // Unless successful, we must be conservative.
+
   vector<string> args = configuration_->GetInitctlArgs(should_be_running);
   unique_ptr<gchar*, GLibStrvFreeDeleter> argv(
       utils::StringVectorToGStrv(args));
@@ -260,29 +266,21 @@
     return false;
   }
 
-  // If initctl(8) exits normally with exit status 0 ("success"), it
-  // meant that it did what we requested.
-  if (WEXITSTATUS(exit_status) == 0) {
-    return true;
-  }
-
-  // Otherwise, screenscape stderr from initctl(8). Ugh, yes, this is
-  // ugly but since the program lacks verbs/actions such as
-  //
-  //  ensure-started (or start-or-return-success-if-already-started)
-  //  ensure-stopped (or stop-or-return-success-if-not-running)
-  //
-  // this is what we have to do.
-  //
+  // If initctl(8) does not exit normally (exit status other than zero), ensure
+  // that the error message is not benign by scanning stderr; this is a
+  // necessity because initctl does not offer actions such as "start if not
+  // running" or "stop if running".
   // TODO(zeuthen,chromium:277051): Avoid doing this.
-  const gchar *expected_error_message = should_be_running ?
-    "initctl: Job is already running: p2p\n" :
-    "initctl: Unknown instance \n";
-  if (g_strcmp0(standard_error, expected_error_message) == 0) {
-    return true;
+  if (WEXITSTATUS(exit_status) != 0) {
+    const gchar *expected_error_message = should_be_running ?
+      "initctl: Job is already running: p2p\n" :
+      "initctl: Unknown instance \n";
+    if (g_strcmp0(standard_error, expected_error_message) != 0)
+      return false;
   }
 
-  return false;
+  may_be_running_ = should_be_running;  // Successful after all.
+  return true;
 }
 
 bool P2PManagerImpl::EnsureP2PRunning() {
@@ -763,6 +761,17 @@
   return num_files;
 }
 
+bool P2PManagerImpl::SetP2PEnabledPref(bool enabled) {
+  if (!prefs_->SetBoolean(chromeos_update_engine::kPrefsP2PEnabled, enabled))
+    return false;
+
+  // If P2P should not be running, make sure it isn't.
+  if (may_be_running_ && !IsP2PEnabled())
+    EnsureP2PNotRunning();
+
+  return true;
+}
+
 P2PManager* P2PManager::Construct(Configuration *configuration,
                                   PrefsInterface *prefs,
                                   const string& file_extension,