Don't save popup blocker whitelist modifications performed while off the record.

BUG=none
TEST=Go incognito, visit popuptest.com, whitelist popups, restart and verify they're not whitelisted.
Review URL: http://codereview.chromium.org/434109

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

2 files changed, 27 insertions, 13 deletions

diff --git a/chrome/browser/blocked_popup_container.cc b/chrome/browser/blocked_popup_container.cc
index e4c2184..bf7a08a 100644
--- a/chrome/browser/blocked_popup_container.cc
+++ b/chrome/browser/blocked_popup_container.cc
@@ -155,11 +155,8 @@ void BlockedPopupContainer::ToggleWhitelistingForHost(size_t index) {
   bool should_whitelist = !i->second;
   popup_hosts_[host] = should_whitelist;
 
-  ListValue* whitelist_pref =
-      prefs_->GetMutableList(prefs::kPopupWhitelistedHosts);
   if (should_whitelist) {
     whitelist_.insert(host);
-    whitelist_pref->Append(new StringValue(host));
 
     // Open the popups in order.
     for (size_t j = 0; j < blocked_popups_.size();) {
@@ -171,8 +168,6 @@ void BlockedPopupContainer::ToggleWhitelistingForHost(size_t index) {
   } else {
     // Remove from whitelist.
     whitelist_.erase(host);
-    StringValue host_as_string(host);
-    whitelist_pref->Remove(host_as_string);
 
     for (UnblockedPopups::iterator i(unblocked_popups_.begin());
          i != unblocked_popups_.end(); ) {
@@ -197,6 +192,21 @@ void BlockedPopupContainer::ToggleWhitelistingForHost(size_t index) {
       }
     }
   }
+
+  // Persist whitelisting state if we're not off the record.
+  if (prefs_) {
+    ListValue* whitelist_pref =
+        prefs_->GetMutableList(prefs::kPopupWhitelistedHosts);
+    if (should_whitelist) {
+      whitelist_pref->Append(new StringValue(host));
+    } else {
+      // Stupidly, gcc complains that you're accessing the (private) StringValue
+      // copy constructor if you inline the temp creation into the Remove()
+      // call.
+      StringValue host_value(host);
+      whitelist_pref->Remove(host_value);
+    }
+  }
 }
 
 void BlockedPopupContainer::CloseAll() {
@@ -391,14 +401,17 @@ BlockedPopupContainer::BlockedPopupContainer(TabContents* owner,
   const ListValue* whitelist_pref =
       prefs_->GetList(prefs::kPopupWhitelistedHosts);
   // Careful: The returned value could be NULL if the pref has never been set.
-  if (whitelist_pref == NULL)
-    return;
-  for (ListValue::const_iterator i(whitelist_pref->begin());
-       i != whitelist_pref->end(); ++i) {
-    std::string host;
-    (*i)->GetAsString(&host);
-    whitelist_.insert(host);
+  if (whitelist_pref != NULL) {
+    for (ListValue::const_iterator i(whitelist_pref->begin());
+         i != whitelist_pref->end(); ++i) {
+      std::string host;
+      (*i)->GetAsString(&host);
+      whitelist_.insert(host);
+    }
   }
+
+  if (profile->IsOffTheRecord())
+    prefs_ = NULL;  // Don't persist whitelist changes.
 }
 
 void BlockedPopupContainer::UpdateView() {
diff --git a/chrome/browser/blocked_popup_container.h b/chrome/browser/blocked_popup_container.h
index 23b4547..2d5ef6a 100644
--- a/chrome/browser/blocked_popup_container.h
+++ b/chrome/browser/blocked_popup_container.h
@@ -278,7 +278,8 @@ class BlockedPopupContainer : public TabContentsDelegate,
   // The TabContents that owns and constrains this BlockedPopupContainer.
   TabContents* owner_;
 
-  // The PrefService we can query to find out what's on the whitelist.
+  // The PrefService we can query to find out what's on the whitelist.  If the
+  // profile is off the record, this will be NULL.
   PrefService* prefs_;
 
   // Once the container is hidden, this is set to prevent it from reappearing.