Revert 222242 "[Activity Log] when extension is uninstalled, del..."

Reverted due to memory leaks.

BUG=288708

> [Activity Log] when extension is uninstalled, delete data about it
> 
> When a user uninstalls an extension, all of the data about that extension is removed from the activity log database. This is a privacy measure.
> 
> BUG=244841
> R=mpcomplete@chromium.org
> 
> Review URL: https://chromiumcodereview.appspot.com/23983014

TBR=felt@chromium.org

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

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

chrome/browser/extensions/activity_log/activity_log.cc

@@ -302,11 +302,6 @@ ActivityLogFactory::ActivityLogFactory()
   DependsOn(InstallTrackerFactory::GetInstance());
 }
 
-// static
-ActivityLog* ActivityLog::GetInstance(Profile* profile) {
-  return ActivityLogFactory::GetForProfile(profile);
-}
-
 ActivityLogFactory::~ActivityLogFactory() {
 }
 
@@ -451,17 +446,18 @@ void ActivityLog::OnExtensionUnloaded(const Extension* extension) {
 void ActivityLog::OnExtensionUninstalled(const Extension* extension) {
   // If the extension has been uninstalled but not disabled, we delete the
   // database.
-  if (extension->id() == kActivityLogExtensionId) {
-    if (!CommandLine::ForCurrentProcess()->HasSwitch(
-        switches::kEnableExtensionActivityLogging)) {
-      DeleteDatabase();
-    }
-  } else {
-    if (policy_)
-      policy_->RemoveExtensionData(extension->id());
+  if (extension->id() != kActivityLogExtensionId) return;
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableExtensionActivityLogging)) {
+    DeleteDatabase();
   }
 }
 
+// static
+ActivityLog* ActivityLog::GetInstance(Profile* profile) {
+  return ActivityLogFactory::GetForProfile(profile);
+}
+
 void ActivityLog::AddObserver(ActivityLog::Observer* observer) {
   observers_->AddObserver(observer);
 }

chrome/browser/extensions/activity_log/activity_log_policy.h

@@ -93,9 +93,6 @@ class ActivityLogPolicy {
   // different policies. If restrict_urls is empty then all URLs are removed.
   virtual void RemoveURLs(const std::vector<GURL>& restrict_urls) = 0;
 
-  // Remove all rows relating to a given extension.
-  virtual void RemoveExtensionData(const std::string& extension_id) = 0;
-
   // Deletes everything in the database.
   virtual void DeleteDatabase() = 0;
 

chrome/browser/extensions/activity_log/activity_log_unittest.cc

@@ -63,12 +63,7 @@ class ActivityLogTest : public ChromeRenderViewHostTestHarness {
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
-  static void RetrieveActions_LogAndFetchActions0(
-      scoped_ptr<std::vector<scoped_refptr<Action> > > i) {
-    ASSERT_EQ(0, static_cast<int>(i->size()));
-  }
-
-  static void RetrieveActions_LogAndFetchActions2(
+  static void RetrieveActions_LogAndFetchActions(
       scoped_ptr<std::vector<scoped_refptr<Action> > > i) {
     ASSERT_EQ(2, static_cast<int>(i->size()));
   }
@@ -161,7 +156,7 @@ TEST_F(ActivityLogTest, LogAndFetchActions) {
       "",
       "",
       0,
-      base::Bind(ActivityLogTest::RetrieveActions_LogAndFetchActions2));
+      base::Bind(ActivityLogTest::RetrieveActions_LogAndFetchActions));
 }
 
 TEST_F(ActivityLogTest, LogPrerender) {
@@ -249,41 +244,4 @@ TEST_F(ActivityLogTest, ArgUrlExtraction) {
       base::Bind(ActivityLogTest::RetrieveActions_ArgUrlExtraction));
 }
 
-TEST_F(ActivityLogTest, UninstalledExtension) {
-  scoped_refptr<const Extension> extension =
-      ExtensionBuilder()
-          .SetManifest(DictionaryBuilder()
-                       .Set("name", "Test extension")
-                       .Set("version", "1.0.0")
-                       .Set("manifest_version", 2))
-          .Build();
-
-  ActivityLog* activity_log = ActivityLog::GetInstance(profile());
-  scoped_ptr<base::ListValue> args(new base::ListValue());
-  ASSERT_TRUE(GetDatabaseEnabled());
-
-  // Write some API calls
-  scoped_refptr<Action> action = new Action(extension->id(),
-                                            base::Time::Now(),
-                                            Action::ACTION_API_CALL,
-                                            "tabs.testMethod");
-  activity_log->LogAction(action);
-  action = new Action(extension->id(),
-                      base::Time::Now(),
-                      Action::ACTION_DOM_ACCESS,
-                      "document.write");
-  action->set_page_url(GURL("http://www.google.com"));
-
-  activity_log->OnExtensionUninstalled(extension);
-
-  activity_log->GetFilteredActions(
-      extension->id(),
-      Action::ACTION_ANY,
-      "",
-      "",
-      "",
-      0,
-      base::Bind(ActivityLogTest::RetrieveActions_LogAndFetchActions0));
-}
-
 }  // namespace extensions

chrome/browser/extensions/activity_log/counting_policy.cc

@@ -579,39 +579,6 @@ void CountingPolicy::DoRemoveURLs(const std::vector<GURL>& restrict_urls) {
   CleanStringTables(db);
 }
 
-void CountingPolicy::DoRemoveExtensionData(const std::string& extension_id) {
-  if (extension_id.empty())
-    return;
-
-  sql::Connection* db = GetDatabaseConnection();
-  if (!db) {
-    LOG(ERROR) << "Unable to connect to database";
-    return;
-  }
-
-  // Make sure any queued in memory are sent to the database before cleaning.
-  activity_database()->AdviseFlush(ActivityDatabase::kFlushImmediately);
-
-  std::string sql_str = base::StringPrintf(
-      "DELETE FROM %s WHERE extension_id_x=?", kTableName);
-  sql::Statement statement(
-      db->GetCachedStatement(sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
-  int64 id;
-  if (string_table_.StringToInt(db, extension_id, &id)) {
-    statement.BindInt64(0, id);
-  } else {
-    // If the string isn't listed, that means we never recorded anything about
-    // the extension so there's no deletion to do.
-    return;
-  }
-  if (!statement.Run()) {
-    LOG(ERROR) << "Removing URLs for extension "
-               << extension_id << "from database failed: "
-               << statement.GetSQLStatement();
-  }
-  CleanStringTables(db);
-}
-
 void CountingPolicy::DoDeleteDatabase() {
   sql::Connection* db = GetDatabaseConnection();
   if (!db) {
@@ -684,10 +651,6 @@ void CountingPolicy::RemoveURLs(const std::vector<GURL>& restrict_urls) {
   ScheduleAndForget(this, &CountingPolicy::DoRemoveURLs, restrict_urls);
 }
 
-void CountingPolicy::RemoveExtensionData(const std::string& extension_id) {
-  ScheduleAndForget(this, &CountingPolicy::DoRemoveExtensionData, extension_id);
-}
-
 void CountingPolicy::DeleteDatabase() {
   ScheduleAndForget(this, &CountingPolicy::DoDeleteDatabase);
 }

chrome/browser/extensions/activity_log/counting_policy.h

@@ -48,9 +48,6 @@ class CountingPolicy : public ActivityLogDatabasePolicy {
   // Clean the URL data stored for this policy.
   virtual void RemoveURLs(const std::vector<GURL>&) OVERRIDE;
 
-  // Clean the data related to this extension for this policy.
-  virtual void RemoveExtensionData(const std::string& extension_id) OVERRIDE;
-
   // Delete everything in the database.
   virtual void DeleteDatabase() OVERRIDE;
 
@@ -92,10 +89,6 @@ class CountingPolicy : public ActivityLogDatabasePolicy {
   // thread.
   void DoRemoveURLs(const std::vector<GURL>& restrict_urls);
 
-  // The implementation of RemoveExtensionData; this must only run on the
-  // database thread.
-  void DoRemoveExtensionData(const std::string& extension_id);
-
   // The implementation of DeleteDatabase; called on the database thread.
   void DoDeleteDatabase();
 

chrome/browser/extensions/activity_log/counting_policy_unittest.cc

@@ -926,63 +926,6 @@ TEST_F(CountingPolicyTest, RemoveSpecificURLs) {
   policy->Close();
 }
 
-TEST_F(CountingPolicyTest, RemoveExtensionData) {
-  CountingPolicy* policy = new CountingPolicy(profile_.get());
-
-  // Use a mock clock to ensure that events are not recorded on the wrong day
-  // when the test is run close to local midnight.
-  base::SimpleTestClock* mock_clock = new base::SimpleTestClock();
-  mock_clock->SetNow(base::Time::Now().LocalMidnight() +
-                     base::TimeDelta::FromHours(12));
-  policy->SetClockForTesting(scoped_ptr<base::Clock>(mock_clock));
-
-  // Record some actions
-  scoped_refptr<Action> action = new Action("deleteextensiondata",
-                                            mock_clock->Now(),
-                                            Action::ACTION_DOM_ACCESS,
-                                            "lets");
-  action->mutable_args()->AppendString("vamoose");
-  action->set_page_title("Google");
-  action->set_arg_url(GURL("http://www.google.com"));
-  policy->ProcessAction(action);
-  policy->ProcessAction(action);
-  policy->ProcessAction(action);
-
-  scoped_refptr<Action> action2 = new Action("dontdelete",
-                                             mock_clock->Now(),
-                                             Action::ACTION_DOM_ACCESS,
-                                             "lets");
-  action->mutable_args()->AppendString("vamoose");
-  action->set_page_title("Google");
-  action->set_arg_url(GURL("http://www.google.com"));
-  policy->ProcessAction(action2);
-
-  policy->Flush();
-  policy->RemoveExtensionData("deleteextensiondata");
-
-  CheckReadFilteredData(
-      policy,
-      "deleteextensiondata",
-      Action::ACTION_ANY,
-      "",
-      "",
-      "",
-      -1,
-      base::Bind(
-          &CountingPolicyTest::RetrieveActions_FetchFilteredActions0));
-
-  CheckReadFilteredData(
-      policy,
-      "dontdelete",
-      Action::ACTION_ANY,
-      "",
-      "",
-      "",
-      -1,
-      base::Bind(
-          &CountingPolicyTest::RetrieveActions_FetchFilteredActions1));
-}
-
 TEST_F(CountingPolicyTest, DeleteActions) {
   CountingPolicy* policy = new CountingPolicy(profile_.get());
   // Disable row expiration for this test by setting a time before any actions

chrome/browser/extensions/activity_log/fullstream_ui_policy.cc

@@ -282,32 +282,6 @@ void FullStreamUIPolicy::DoRemoveURLs(const std::vector<GURL>& restrict_urls) {
   }
 }
 
-void FullStreamUIPolicy::DoRemoveExtensionData(
-    const std::string& extension_id) {
-  if (extension_id.empty())
-    return;
-
-  sql::Connection* db = GetDatabaseConnection();
-  if (!db) {
-    LOG(ERROR) << "Unable to connect to database";
-    return;
-  }
-
-  // Make sure any queued in memory are sent to the database before cleaning.
-  activity_database()->AdviseFlush(ActivityDatabase::kFlushImmediately);
-
-  std::string sql_str = base::StringPrintf(
-      "DELETE FROM %s WHERE extension_id=?", kTableName);
-  sql::Statement statement(
-      db->GetCachedStatement(sql::StatementID(SQL_FROM_HERE), sql_str.c_str()));
-  statement.BindString(0, extension_id);
-  if (!statement.Run()) {
-    LOG(ERROR) << "Removing URLs for extension "
-               << extension_id << "from database failed: "
-               << statement.GetSQLStatement();
-  }
-}
-
 void FullStreamUIPolicy::DoDeleteDatabase() {
   sql::Connection* db = GetDatabaseConnection();
   if (!db) {
@@ -377,11 +351,6 @@ void FullStreamUIPolicy::RemoveURLs(const std::vector<GURL>& restrict_urls) {
   ScheduleAndForget(this, &FullStreamUIPolicy::DoRemoveURLs, restrict_urls);
 }
 
-void FullStreamUIPolicy::RemoveExtensionData(const std::string& extension_id) {
-  ScheduleAndForget(
-      this, &FullStreamUIPolicy::DoRemoveExtensionData, extension_id);
-}
-
 void FullStreamUIPolicy::DeleteDatabase() {
   ScheduleAndForget(this, &FullStreamUIPolicy::DoDeleteDatabase);
 }

chrome/browser/extensions/activity_log/fullstream_ui_policy.h

@@ -43,9 +43,6 @@ class FullStreamUIPolicy : public ActivityLogDatabasePolicy {
   // Clean the URL data stored for this policy.
   virtual void RemoveURLs(const std::vector<GURL>& restrict_urls) OVERRIDE;
 
-  // Clean the data related to this extension for this policy.
-  virtual void RemoveExtensionData(const std::string& extension_id) OVERRIDE;
-
   // Delete everything in the database.
   virtual void DeleteDatabase() OVERRIDE;
 
@@ -77,10 +74,6 @@ class FullStreamUIPolicy : public ActivityLogDatabasePolicy {
   // thread.
   void DoRemoveURLs(const std::vector<GURL>& restrict_urls);
 
-  // The implementation of RemoveExtensionData; this must only run on the
-  // database thread.
-  void DoRemoveExtensionData(const std::string& extension_id);
-
   // The implementation of DeleteDatabase; this must only run on the database
   // thread.
   void DoDeleteDatabase();

chrome/browser/extensions/activity_log/fullstream_ui_policy_unittest.cc

@@ -633,63 +633,6 @@ TEST_F(FullStreamUIPolicyTest, RemoveSpecificURLs) {
   policy->Close();
 }
 
-TEST_F(FullStreamUIPolicyTest, RemoveExtensionData) {
-  FullStreamUIPolicy* policy = new FullStreamUIPolicy(profile_.get());
-
-  // Use a mock clock to ensure that events are not recorded on the wrong day
-  // when the test is run close to local midnight.
-  base::SimpleTestClock* mock_clock = new base::SimpleTestClock();
-  mock_clock->SetNow(base::Time::Now().LocalMidnight() +
-                     base::TimeDelta::FromHours(12));
-  policy->SetClockForTesting(scoped_ptr<base::Clock>(mock_clock));
-
-  // Record some actions
-  scoped_refptr<Action> action = new Action("deleteextensiondata",
-                                            mock_clock->Now(),
-                                            Action::ACTION_DOM_ACCESS,
-                                            "lets");
-  action->mutable_args()->AppendString("vamoose");
-  action->set_page_title("Google");
-  action->set_arg_url(GURL("http://www.google.com"));
-  policy->ProcessAction(action);
-  policy->ProcessAction(action);
-  policy->ProcessAction(action);
-
-  scoped_refptr<Action> action2 = new Action("dontdelete",
-                                             mock_clock->Now(),
-                                             Action::ACTION_DOM_ACCESS,
-                                             "lets");
-  action->mutable_args()->AppendString("vamoose");
-  action->set_page_title("Google");
-  action->set_arg_url(GURL("http://www.google.com"));
-  policy->ProcessAction(action2);
-
-  policy->Flush();
-  policy->RemoveExtensionData("deleteextensiondata");
-
-  CheckReadFilteredData(
-      policy,
-      "deleteextensiondata",
-      Action::ACTION_ANY,
-      "",
-      "",
-      "",
-      -1,
-      base::Bind(
-          &FullStreamUIPolicyTest::RetrieveActions_FetchFilteredActions0));
-
-  CheckReadFilteredData(
-      policy,
-      "dontdelete",
-      Action::ACTION_ANY,
-      "",
-      "",
-      "",
-      -1,
-      base::Bind(
-          &FullStreamUIPolicyTest::RetrieveActions_FetchFilteredActions1));
-}
-
 TEST_F(FullStreamUIPolicyTest, CapReturns) {
   FullStreamUIPolicy* policy = new FullStreamUIPolicy(profile_.get());