Closing a guest session clears all browsing history.

chrome/browser/ui/browser.cc

Index: chrome/browser/ui/browser.cc
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index f438efd2273adeda29b552ea373597101c6020ed..149ee1ff8c4228340f804bf6ec78c54fcb26720b 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -34,6 +34,8 @@
 #include "chrome/browser/background/background_contents_service_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_shutdown.h"
+#include "chrome/browser/browsing_data/browsing_data_helper.h"
+#include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/character_encoding.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
@@ -511,9 +513,20 @@ Browser::~Browser() {
 
   if (profile_->IsOffTheRecord() &&
       !BrowserList::IsOffTheRecordSessionActiveForProfile(profile_)) {
-    // An incognito profile is no longer needed, this indirectly frees
-    // its cache and cookies once it gets destroyed at the appropriate time.
-    ProfileDestroyer::DestroyProfileWhenAppropriate(profile_);
+    if (profile_->IsGuestSession()) {

[Peter Kasting, 2015/02/10 21:43:36]

How come the incognito codepath doesn't actually work right?  The bug sounds
like this causes us to delete more state than before, but from the way the code
here reads I'd expect it to delete less state -- the incognito path destroys the
entire profile while this doesn't (and it even has a TODO to do so).

It seems like maybe there should be more comments here about why we can't do
what the incognito path does. 

[Mike Lerman, 2015/02/10 21:53:32]

I've modified the comment to explain why we're doing this. In short, the guest
profile has more setup than the incognito profile (such as
BrowserContextKeyedServices), and deleting it is hard, and won't get merged into
M41.

Ideally, we want to delete as much state as possible, but that requires profile
deletion, so for now we're going to clear browser data.

[Peter Kasting, 2015/02/10 21:58:10]

OK, the new comment is better, but I still don't really know what "it's harder
to just delete it" means.  Does that mean "DestroyProfileWhenAppropriate()
doesn't work right, and the fix for that is too big to merge, but we're going to
do that and then remove the conditional here again"?  Or something else?

[Mike Lerman, 2015/02/11 15:15:32]

You're exactly right. DestroyProfileWhenAppropriate, and the ProfileDestroyer,
just wrap and delay around deleting the Profile*. Deleting a Profile* (except
for an offTheRecordProfile) while Chrome's running doesn't currently work, and
that fix will be too big to merge.

I'm working on that fix (https://codereview.chromium.org/853743002/), and when I
lands that I can remove the BrowsingDataRemover here and just use the
ProfileDestroyer, as does the OffTheRecord.

Changed the comment a bit more.

+      // Clear all browsing data once a Guest Session completes.
+      // TODO(mlerman): Delete the guest |profile_| completely instead.
+      BrowsingDataRemover* data_remover =
+          BrowsingDataRemover::CreateForUnboundedRange(profile_);
+      data_remover->Remove(BrowsingDataRemover::REMOVE_ALL ^
+                               BrowsingDataRemover::REMOVE_DOWNLOADS,
+                           BrowsingDataHelper::ALL);
+      // BrowsingDataRemover deletes itself.
+    } else {
+      // An incognito profile is no longer needed, this indirectly frees
+      // its cache and cookies once it gets destroyed at the appropriate time.
+      ProfileDestroyer::DestroyProfileWhenAppropriate(profile_);
+    }
   }
 
   // There may be pending file dialogs, we need to tell them that we've gone