memlog: respect Incognito mode and Metrics collection settings.

Do not upload memory traces if there is an active incognito session
or if the user has opted out of metrics collections.

Bug: 785068
Change-Id: If5f00792a6d1306d77c36cbd88066d39c7dbe146
Reviewed-on: https://chromium-review.googlesource.com/775735Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517263}

chrome/browser/metrics/chrome_metrics_service_accessor.h

@@ -51,6 +51,10 @@ namespace prerender {
 bool IsOmniboxEnabled(Profile* profile);
 }
 
+namespace profiling {
+class BackgroundProfilingTriggers;
+}
+
 namespace safe_browsing {
 class ChromeCleanerControllerDelegate;
 class DownloadUrlSBClient;
@@ -108,6 +112,7 @@ class ChromeMetricsServiceAccessor : public metrics::MetricsServiceAccessor {
       const OnMetricsReportingCallbackType&);
   friend class options::BrowserOptionsHandler;
   friend bool prerender::IsOmniboxEnabled(Profile* profile);
+  friend class profiling::BackgroundProfilingTriggers;
   friend class settings::MetricsReportingHandler;
   friend class speech::ChromeSpeechRecognitionManagerDelegate;
   friend class system_logs::ChromeInternalLogSource;

chrome/browser/profiling_host/background_profiling_triggers.cc

@@ -6,6 +6,10 @@
 
 #include "base/task_scheduler/post_task.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiling_host/profiling_process_host.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/process_type.h"
@@ -76,6 +80,23 @@ void BackgroundProfilingTriggers::StartTimer() {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+bool BackgroundProfilingTriggers::IsAllowedToUpload() const {
+  if (!ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled()) {
+    return false;
+  }
+
+  // Do not upload if there is an incognito session running in any profile.
+  std::vector<Profile*> profiles =
+      g_browser_process->profile_manager()->GetLoadedProfiles();
+  for (Profile* profile : profiles) {
+    if (profile->HasOffTheRecordProfile()) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 bool BackgroundProfilingTriggers::IsOverTriggerThreshold(
     int content_process_type,
     uint32_t private_footprint_kb) {
@@ -101,6 +122,10 @@ bool BackgroundProfilingTriggers::IsOverTriggerThreshold(
 void BackgroundProfilingTriggers::PerformMemoryUsageChecks() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
+  if (!IsAllowedToUpload()) {
+    return;
+  }
+
   memory_instrumentation::MemoryInstrumentation::GetInstance()
       ->RequestGlobalDump(
           base::Bind(&BackgroundProfilingTriggers::OnReceivedMemoryDump,

chrome/browser/profiling_host/background_profiling_triggers.h

@@ -19,6 +19,10 @@ class ProfilingProcessHost;
 // collection of memory dumps and upload the results to the slow-reports
 // service. BackgroundProfilingTriggers class sets a periodic timer and
 // interacts with ProfilingProcessHost to trigger and upload memory dumps.
+//
+// When started, memory information is collected every hour to check if any
+// process is over the trigger threshold. Once a report is uploaded, the
+// collection interval is changed to once every 12 hours.
 class BackgroundProfilingTriggers {
  public:
   explicit BackgroundProfilingTriggers(ProfilingProcessHost* host);
@@ -29,6 +33,13 @@ class BackgroundProfilingTriggers {
 
  private:
   friend class FakeBackgroundProfilingTriggers;
+  FRIEND_TEST_ALL_PREFIXES(BackgroundProfilingTriggersTest,
+                           IsAllowedToUpload_Metrics);
+  FRIEND_TEST_ALL_PREFIXES(BackgroundProfilingTriggersTest,
+                           IsAllowedToUpload_Incognito);
+
+  // Returns true if trace uploads are allowed.
+  bool IsAllowedToUpload() const;
 
   // Returns true if |private_footprint_kb| is large enough to trigger
   // a report for the given |content_process_type|.

chrome/browser/profiling_host/background_profiling_triggers_unittest.cc

@@ -10,7 +10,11 @@
 
 #include "base/test/scoped_task_environment.h"
 #include "base/time/time.h"
+#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/profiling_host/profiling_process_host.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "services/resource_coordinator/public/cpp/memory_instrumentation/coordinator.h"
 #include "services/resource_coordinator/public/interfaces/memory_instrumentation/memory_instrumentation.mojom.h"
@@ -94,18 +98,34 @@ class FakeBackgroundProfilingTriggers : public BackgroundProfilingTriggers {
 class BackgroundProfilingTriggersTest : public testing::Test {
  public:
   BackgroundProfilingTriggersTest()
-      : scoped_task_environment_(
+      : scoped_task_envrionment_(
             base::test::ScopedTaskEnvironment::MainThreadType::UI),
-        triggers_(&host_) {}
+        testing_profile_manager_(TestingBrowserProcess::GetGlobal()),
+        triggers_(&host_),
+        is_metrics_enabled_(true) {}
+
+  void SetUp() override {
+    ASSERT_TRUE(testing_profile_manager_.SetUp());
+    ChromeMetricsServiceAccessor::SetMetricsAndCrashReportingForTesting(
+        &is_metrics_enabled_);
+  }
+
+  void TearDown() override {
+    ChromeMetricsServiceAccessor::SetMetricsAndCrashReportingForTesting(
+        nullptr);
+  }
 
   void SetMode(ProfilingProcessHost::Mode mode) { host_.SetMode(mode); }
 
  protected:
-  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  base::test::ScopedTaskEnvironment scoped_task_envrionment_;
   content::TestBrowserThreadBundle thread_bundle;
+  TestingProfileManager testing_profile_manager_;
 
   ProfilingProcessHost host_;
   FakeBackgroundProfilingTriggers triggers_;
+
+  bool is_metrics_enabled_;
 };
 
 // Ensures:
@@ -202,4 +222,59 @@ TEST_F(BackgroundProfilingTriggersTest,
   //  http://crbug.com/780955
 }
 
+// Ensure IsAllowedToUpload() respects metrics collection settings.
+TEST_F(BackgroundProfilingTriggersTest, IsAllowedToUpload_Metrics) {
+  EXPECT_TRUE(triggers_.IsAllowedToUpload());
+
+  is_metrics_enabled_ = false;
+  EXPECT_FALSE(triggers_.IsAllowedToUpload());
+}
+
+// Ensure IsAllowedToUpload() respects incognito sessions. Checke that behavior
+//   * respsects incognito sessions in primary profile.
+//   * respsects incognito sessions in non-primary profiles.
+//   * handles overlapping incognito sessions.
+//
+// NOTE: As of this test writing, TestingProfile::DestroyOffTheRecordProfile()
+// is mocked out to do nothing. Currently, using
+// TestingProfile::SetOffTheRecordProfile(nullptr) to fake destruction.
+TEST_F(BackgroundProfilingTriggersTest, IsAllowedToUpload_Incognito) {
+  // Create 2 profiles. The first is considered the primary.
+  TestingProfile* primary_profile =
+      testing_profile_manager_.CreateTestingProfile("primary");
+  TestingProfile* secondary_profile =
+      testing_profile_manager_.CreateTestingProfile("secondary");
+  ASSERT_FALSE(primary_profile->HasOffTheRecordProfile());
+
+  // Test IsAllowedToUpload() maps to incognito session in primary profile.
+  EXPECT_TRUE(triggers_.IsAllowedToUpload());
+
+  primary_profile->GetOffTheRecordProfile();
+  EXPECT_FALSE(triggers_.IsAllowedToUpload());
+
+  primary_profile->SetOffTheRecordProfile(nullptr);
+  EXPECT_TRUE(triggers_.IsAllowedToUpload());
+
+  // Test IsAllowedToUpload() maps to incognito session in secondary profile.
+  secondary_profile->GetOffTheRecordProfile();
+  EXPECT_FALSE(triggers_.IsAllowedToUpload());
+  secondary_profile->SetOffTheRecordProfile(nullptr);
+  EXPECT_TRUE(triggers_.IsAllowedToUpload());
+
+  // Test overlapping incognitos sessions.
+  primary_profile->GetOffTheRecordProfile();
+  secondary_profile->GetOffTheRecordProfile();
+  EXPECT_FALSE(triggers_.IsAllowedToUpload());
+
+  secondary_profile->SetOffTheRecordProfile(nullptr);
+  EXPECT_FALSE(triggers_.IsAllowedToUpload());
+
+  secondary_profile->GetOffTheRecordProfile();
+  primary_profile->SetOffTheRecordProfile(nullptr);
+  EXPECT_FALSE(triggers_.IsAllowedToUpload());
+
+  secondary_profile->SetOffTheRecordProfile(nullptr);
+  EXPECT_TRUE(triggers_.IsAllowedToUpload());
+}
+
 }  // namespace profiling