Adding "Clear hosted app data" checkbox to browsing data removal UI.

chrome/browser/ui/webui/options2/clear_browser_data_handler2.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/options2/clear_browser_data_handler2.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/string16.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data_helper.h"
 #include "chrome/browser/browsing_data_remover.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/web_ui.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace options2 {
 
-ClearBrowserDataHandler::ClearBrowserDataHandler() : remover_(NULL) {
+ClearBrowserDataHandler::ClearBrowserDataHandler()
+    : remover_(NULL),
+      remove_hosted_app_data_pending_(false) {
 }
 
 ClearBrowserDataHandler::~ClearBrowserDataHandler() {
   if (remover_)
     remover_->RemoveObserver(this);
 }
 
 void ClearBrowserDataHandler::InitializeHandler() {
   clear_plugin_lso_data_enabled_.Init(prefs::kClearPluginLSODataEnabled,
                                       Profile::FromWebUI(web_ui())->GetPrefs(),
                                       NULL);
 }
 
 void ClearBrowserDataHandler::GetLocalizedValues(
     DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
 
   static OptionsStringResource resources[] = {
     { "clearBrowserDataLabel", IDS_CLEAR_BROWSING_DATA_LABEL },
     { "deleteBrowsingHistoryCheckbox", IDS_DEL_BROWSING_HISTORY_CHKBOX },
     { "deleteDownloadHistoryCheckbox", IDS_DEL_DOWNLOAD_HISTORY_CHKBOX },
     { "deleteCacheCheckbox", IDS_DEL_CACHE_CHKBOX },
     { "deleteCookiesCheckbox", IDS_DEL_COOKIES_CHKBOX },
     { "deleteCookiesFlashCheckbox", IDS_DEL_COOKIES_FLASH_CHKBOX },
     { "deletePasswordsCheckbox", IDS_DEL_PASSWORDS_CHKBOX },
     { "deleteFormDataCheckbox", IDS_DEL_FORM_DATA_CHKBOX },
+    { "deleteHostedAppsDataCheckbox", IDS_DEL_HOSTED_APPS_DATA_CHKBOX },
     { "deauthorizeContentLicensesCheckbox",
       IDS_DEAUTHORIZE_CONTENT_LICENSES_CHKBOX },
     { "clearBrowserDataCommit", IDS_CLEAR_BROWSING_DATA_COMMIT },
     { "flash_storage_url", IDS_FLASH_STORAGE_URL },
   };
 
   RegisterStrings(localized_strings, resources, arraysize(resources));
   RegisterTitle(localized_strings, "clearBrowserDataOverlay",
                 IDS_CLEAR_BROWSING_DATA_TITLE);
 
@@ skipping 21 matching lines @@
     option->Append(Value::CreateIntegerValue(i));
     option->Append(Value::CreateStringValue(label_string));
     time_list->Append(option);
   }
   localized_strings->Set("clearBrowserDataTimeList", time_list);
 }
 
 void ClearBrowserDataHandler::RegisterMessages() {
   // Setup handlers specific to this panel.
   web_ui()->RegisterMessageCallback("performClearBrowserData",
-      base::Bind(&ClearBrowserDataHandler::HandleClearBrowserData,
+      base::Bind(&ClearBrowserDataHandler::ClearBrowserData,
                  base::Unretained(this)));
 }
 
-void ClearBrowserDataHandler::HandleClearBrowserData(const ListValue* value) {
+void ClearBrowserDataHandler::ClearBrowserData(const ListValue* value) {

[Evan Stade, 2012/06/05 18:07:45]

this one actually does handle a chrome.send message (see L98), so it should
start with Handle.

[Mike West, 2012/06/11 09:35:44]

Done.

   Profile* profile = Profile::FromWebUI(web_ui());
   PrefService* prefs = profile->GetPrefs();
 
   int remove_mask = 0;
   if (prefs->GetBoolean(prefs::kDeleteBrowsingHistory))
     remove_mask |= BrowsingDataRemover::REMOVE_HISTORY;
   if (prefs->GetBoolean(prefs::kDeleteDownloadHistory))
     remove_mask |= BrowsingDataRemover::REMOVE_DOWNLOADS;
   if (prefs->GetBoolean(prefs::kDeleteCache))
     remove_mask |= BrowsingDataRemover::REMOVE_CACHE;
   if (prefs->GetBoolean(prefs::kDeleteCookies)) {
     int site_data_mask = BrowsingDataRemover::REMOVE_SITE_DATA;
     // Don't try to clear LSO data if it's not supported.
     if (!*clear_plugin_lso_data_enabled_)
       site_data_mask &= ~BrowsingDataRemover::REMOVE_PLUGIN_DATA;
     remove_mask |= site_data_mask;
   }
   if (prefs->GetBoolean(prefs::kDeletePasswords))
     remove_mask |= BrowsingDataRemover::REMOVE_PASSWORDS;
   if (prefs->GetBoolean(prefs::kDeleteFormData))
     remove_mask |= BrowsingDataRemover::REMOVE_FORM_DATA;
   if (prefs->GetBoolean(prefs::kDeauthorizeContentLicenses))
     remove_mask |= BrowsingDataRemover::REMOVE_CONTENT_LICENSES;
 
+  remove_hosted_app_data_pending_ =
+      prefs->GetBoolean(prefs::kDeleteHostedAppsData);
+
   int period_selected = prefs->GetInteger(prefs::kDeleteTimePeriod);

[Evan Stade, 2012/06/05 18:07:45]

this should be declared where it's used.

[Mike West, 2012/06/11 09:35:44]

Done.

 
   base::FundamentalValue state(true);
   web_ui()->CallJavascriptFunction("ClearBrowserDataOverlay.setClearingState",
                                    state);

[Evan Stade, 2012/06/05 18:07:45]

(read this comment after the one right below)

OK, I think I see why you could get two performClearBrowserData messages right
in a row: we don't disable the UI until here. I cannot fathom why. Should remove
this completely and do setClearingState(true) in
$('clearBrowserDataCommit').onclick.

[Mike West, 2012/06/11 09:35:44]

Good catch. I've moved this out into the JavaScript file.

 
   // If we are still observing a previous data remover, we need to stop
   // observing.
   if (remover_)
     remover_->RemoveObserver(this);

[Evan Stade, 2012/06/05 18:07:45]

I don't really like this pattern. How would (Handle)ClearBrowserData get called
again when it already has a |remover_|? It should be impossible because the UI
is disabled until the first removal is complete.

The problem is that now that OnBrowsingDataRemoverDone does something important
(i.e. continues removing more data that the user has chosen to remove), ignoring
the previous remover_ now has real consequences.

Since I don't understand how remover_ could legitimately be non-NULL here, I'd
just as soon return early.

[Mike West, 2012/06/11 09:35:44]

If `remover_` is non-NULL, then something unexpected has happened. Since it
shouldn't ever happen, I think a DCHECK is appropriate here.

 
   // BrowsingDataRemover deletes itself when done.
   remover_ = new BrowsingDataRemover(profile,
       static_cast<BrowsingDataRemover::TimePeriod>(period_selected),
       base::Time());
   remover_->AddObserver(this);
   remover_->Remove(remove_mask, BrowsingDataHelper::UNPROTECTED_WEB);

[Evan Stade, 2012/06/05 18:07:45]

it seems that remove_mask can be 0 here, in which case this call is a bit
pointless.

[battre, 2012/06/07 21:17:24]

We rely on the callback even if remove_mask is 0 here to trigger
ClearHostedAppData().

[Evan Stade, 2012/06/07 21:50:21]

yea, I can see that. Doesn't mean it makes sense. Instead we should jump
straight to ClearHostedAppData().

[Mike West, 2012/06/11 09:35:44]

You're right. I've added a skip. I've also handled the case where no checkboxes
are checked (which JS shouldn't allow, but better safe than sorry).

 }
 
+void ClearBrowserDataHandler::ClearHostedAppData() {
+  Profile* profile = Profile::FromWebUI(web_ui());
+  PrefService* prefs = profile->GetPrefs();
+
+  // If we are still observing a previous data remover, we need to stop
+  // observing.
+  if (remover_)

[Evan Stade, 2012/06/05 18:07:45]

this is still here. intentional?

[battre, 2012/06/07 21:17:24]

This was just for defensive programming. There is currently no code path in
which remover_ would be != NULL.

[Evan Stade, 2012/06/07 21:50:21]

then DCHECK(!remover_) is more appropriate

[Mike West, 2012/06/11 09:35:44]

Done.

+    remover_->RemoveObserver(this);
+
+  int period_selected = prefs->GetInteger(prefs::kDeleteTimePeriod);
+  // BrowsingDataRemover deletes itself when done.
+  remover_ = new BrowsingDataRemover(
+      profile,
+      static_cast<BrowsingDataRemover::TimePeriod>(period_selected),
+      base::Time());
+  remover_->AddObserver(this);
+  remover_->Remove(BrowsingDataRemover::REMOVE_SITE_DATA,
+                   BrowsingDataHelper::PROTECTED_WEB);
+}
+
 void ClearBrowserDataHandler::OnBrowsingDataRemoverDone() {
   // No need to remove ourselves as an observer as BrowsingDataRemover deletes
   // itself after we return.
   remover_ = NULL;
-  web_ui()->CallJavascriptFunction("ClearBrowserDataOverlay.doneClearing");
+
+  if (remove_hosted_app_data_pending_) {
+    remove_hosted_app_data_pending_ = false;
+    ClearHostedAppData();
+  } else {
+    web_ui()->CallJavascriptFunction("ClearBrowserDataOverlay.doneClearing");
+  }
 }
 
 }  // namespace options2