Modify clear-data-on-exit flag

[josephnz]

can chrome://flags/#clear-data-on-exit be modified to either select what data is removed (this is possible with Brave browser) or at-least exclude

1. cookies
2. site configuration data

In its current form all data including history, cookies etc are removed on browser close which makes it impossible to 'whitelist' website that are allowed to save cookies, such as website that I want to store login information for. If this feature is tweaked to match features available on brave browser, one can selectively delete all site data. A screenshot of the brave setting page is attached

[PF4Public]

Have you tried Cookie AutoDelete? Duplicate of #1745?

[josephnz]

Cookie AutoDelete does not always cleanup  all the non whitelisted cookies. It leaves some behind. On 21 Apr 2022 21:50, PF4Public @.***> wrote: Have you tried Cookie AutoDelete? Duplicate of #1745?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

[PF4Public]

does not always cleanup  all the non whitelisted cookies. It leaves some behind

Probably a candidate for their bugtracker instead?

[josephnz]

Thanks for the suggestion. But I'm more interested in a browser native solution as shown in the screenshot. Firefox also has a similar built-in feature.BTW, someone has already raised a bug with the CAD team. I don't have the link handy.On 22 Apr 2022 07:53, PF4Public @.***> wrote:

does not always cleanup  all the non whitelisted cookies. It leaves some behind

Probably a candidate for their bugtracker instead?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

[ghost]

Chromium takes far too long to compile for me to write this patch myself but I would love for this feature to be included in ungoogled chromium. If anyone else is willing to work on it, the following is a pull request for brave adding 'clear on exit' to the clear history context menu.

https://github.com/brave/brave-core/pull/1213/files

[PF4Public]

the following is a pull request for brave adding 'clear on exit' to the clear history context menu.

https://github.com/brave/brave-core/pull/1213/files

The code is licensed under MPL — a thing to consider.

[benjamasu]

Maybe at least implement this without a fancy interface, but just via feature flags (for example, kClearSiteSettingsOnExit, kClearDownloadsOnExit and so on) and make kClearDataOnExit a meta flag? Perhaps it is worth naming them otherwise, but the point doesn't change.

[PF4Public]

but the point doesn't change

The point indeed does not change. Someone has to implement that.

[benjamasu]

I'm a novice, but still decided to try to roughly adapt an existing patch. Note, that it hasn't been tested in any way.

--- a/chrome/browser/browser_features.cc
+++ b/chrome/browser/browser_features.cc
@@ -410,4 +410,12 @@ BASE_FEATURE(kReportPakFileIntegrity,
 BASE_FEATURE(kRemovalOfIWAsFromTabCapture,
              "RemovalOfIWAsFromTabCapture",
              base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kClearDataOnExit, "ClearDataOnExit", base::FEATURE_DISABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearCacheOnExit, "AllowClearCacheOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearDownloadsOnExit, "AllowClearDownloadsOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearSiteSettingsOnExit, "AllowClearSiteSettingsOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearFormDataOnExit, "AllowClearFormDataOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearPasswordsOnExit, "AllowClearPasswordsOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearHistoryOnExit, "AllowClearHistoryOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
+BASE_FEATURE(kAllowClearSiteDataOnExit, "AllowClearSiteDataOnExit", base::FEATURE_ENABLED_BY_DEFAULT);
 }  // namespace features
--- a/chrome/browser/browser_features.h
+++ b/chrome/browser/browser_features.h
@@ -145,6 +145,14 @@ BASE_DECLARE_FEATURE(kBrowserDynamicCode
 BASE_DECLARE_FEATURE(kReportPakFileIntegrity);

 BASE_DECLARE_FEATURE(kRemovalOfIWAsFromTabCapture);
+BASE_DECLARE_FEATURE(kClearDataOnExit);
+BASE_DECLARE_FEATURE(kAllowClearCacheOnExit);
+BASE_DECLARE_FEATURE(kAllowClearDownloadsOnExit);
+BASE_DECLARE_FEATURE(kAllowClearSiteSettingsOnExit);
+BASE_DECLARE_FEATURE(kAllowClearFormDataOnExit);
+BASE_DECLARE_FEATURE(kAllowClearPasswordsOnExit);
+BASE_DECLARE_FEATURE(kAllowClearHistoryOnExit);
+BASE_DECLARE_FEATURE(kAllowClearSiteDataOnExit);
 }  // namespace features

 #endif  // CHROME_BROWSER_BROWSER_FEATURES_H_
--- a/chrome/browser/browsing_data/chrome_browsing_data_lifetime_manager.cc
+++ b/chrome/browser/browsing_data/chrome_browsing_data_lifetime_manager.cc
@@ -19,6 +19,7 @@
 #include "base/task/task_traits.h"
 #include "base/values.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_features.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_constants.h"
 #include "chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h"
@@ -147,6 +148,21 @@ class BrowsingDataRemoverObserver
 #endif
 };

+uint64_t AllOriginTypeMask() {
+  return content::BrowsingDataRemover::ORIGIN_TYPE_PROTECTED_WEB |
+         content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB;
+}
+
+uint64_t ClearOnExitMask() {
+  return (base::FeatureList::IsEnabled(features::kAllowClearCacheOnExit) ? content::BrowsingDataRemover::DATA_TYPE_CACHE : 0) |
+         (base::FeatureList::IsEnabled(features::kAllowClearDownloadsOnExit) ? content::BrowsingDataRemover::DATA_TYPE_DOWNLOADS : 0) |
+         (base::FeatureList::IsEnabled(features::kAllowClearSiteSettingsOnExit) ? chrome_browsing_data_remover::DATA_TYPE_CONTENT_SETTINGS : 0) |
+         (base::FeatureList::IsEnabled(features::kAllowClearFormDataOnExit) ? chrome_browsing_data_remover::DATA_TYPE_FORM_DATA : 0) |
+         (base::FeatureList::IsEnabled(features::kAllowClearHistoryOnExit) ? chrome_browsing_data_remover::DATA_TYPE_HISTORY : 0) |
+         (base::FeatureList::IsEnabled(features::kAllowClearPasswordsOnExit) ? chrome_browsing_data_remover::DATA_TYPE_PASSWORDS : 0) |
+         (base::FeatureList::IsEnabled(features::kAllowClearSiteDataOnExit) ? chrome_browsing_data_remover::DATA_TYPE_SITE_DATA : 0);
+}
+
 uint64_t GetOriginTypeMask(const base::Value::List& data_types) {
   uint64_t result = 0;
   for (const auto& data_type : data_types) {
@@ -314,9 +330,10 @@ void ChromeBrowsingDataLifetimeManager::
   const base::Value::List& data_types = profile_->GetPrefs()->GetList(
       browsing_data::prefs::kClearBrowsingDataOnExitList);

-  if (!data_types.empty() &&
+  bool cdoe = base::FeatureList::IsEnabled(features::kClearDataOnExit);
+  if (cdoe || (!data_types.empty() &&
       IsConditionSatisfiedForBrowsingDataRemoval(GetSyncTypesForPolicyPref(
-          profile_, browsing_data::prefs::kClearBrowsingDataOnExitList))) {
+          profile_, browsing_data::prefs::kClearBrowsingDataOnExitList)))) {
     profile_->GetPrefs()->SetBoolean(
         browsing_data::prefs::kClearBrowsingDataOnExitDeletionPending, true);
     auto* remover = profile_->GetBrowsingDataRemover();
@@ -327,8 +344,8 @@ void ChromeBrowsingDataLifetimeManager::
       DCHECK(keep_browser_alive);
 #endif
     remover->RemoveAndReply(base::Time(), base::Time::Max(),
-                            GetRemoveMask(data_types),
-                            GetOriginTypeMask(data_types),
+                            cdoe ? ClearOnExitMask() : GetRemoveMask(data_types),
+                            cdoe ? AllOriginTypeMask() : GetOriginTypeMask(data_types),
                             BrowsingDataRemoverObserver::Create(
                                 remover, /*filterable_deletion=*/true, profile_,
                                 keep_browser_alive));
--- a/chrome/browser/ungoogled_flag_entries.h
+++ b/chrome/browser/ungoogled_flag_entries.h
@@ -56,3 +56,35 @@
      "Keep old history",
      "Keep history older than 3 months. ungoogled-chromium flag",
      kOsAll, SINGLE_VALUE_TYPE("keep-old-history")},
+    {"clear-data-on-exit",
+     "Clear data on exit",
+     "Clears selected browser data on exit. See #allow-clearing-*-on-exit flags for tuning. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kClearDataOnExit)},
+    {"allow-clearing-cache-on-exit",
+     "Allow clearing cache on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearCacheOnExit)},
+    {"allow-clearing-downloads-on-exit",
+     "Allow clearing downloads list on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearDownloadsOnExit)},
+    {"allow-clearing-site-settings-on-exit",
+     "Allow clearing site settings on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearSiteSettingsOnExit)},
+    {"allow-clearing-form-data-on-exit",
+     "Allow clearing form data on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearFormDataOnExit)},
+    {"allow-clearing-passwords-on-exit",
+     "Allow clearing saved credentials on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearPasswordsOnExit)},
+    {"allow-clearing-history-on-exit",
+     "Allow clearing history on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearHistoryOnExit)},
+    {"allow-clearing-site-data-on-exit",
+     "Allow clearing site data on exit",
+     "See #clear-data-on-exit flag. ungoogled-chromium flag",
+     kOsDesktop, FEATURE_VALUE_TYPE(features::kAllowClearSiteDataOnExit)},

#endif  // CHROME_BROWSER_UNGOOGLED_FLAG_ENTRIES_H_

[PF4Public]

it hasn't been tested in any way

Which would make a whole lot of a difference. Testing is part of implementation, another important part of it being maintenance. Testing is actually the most time consuming part to be honest.

[benjamasu]

@PF4Public I agree with you on this.

I have successfully built ungoogled-chromium with these changes and at least it works as intended.

https://github.com/benjamasu/ungoogled-chromium/commit/43b3b1db1d2b71bb7ef134730f3582f7466a2f1c

[PF4Public]

@Ahrotahn @networkException Are the proposed changes good enough for a PR?

[benjamasu]

I made the description of the flags a bit more detailed and added their description to docs/flags.md.

https://github.com/benjamasu/ungoogled-chromium/compare/master...benjamasu:ungoogled-chromium:configurable-clear-on-exit

[Ahrotahn]

I don't see why not, you don't need our permission to make a PR!

[benjamasu]

Before submitting a pull request, I want to try to implement the ability to add exceptions via site settings.

I noticed that clear-on-exit only works when Chromium is explicitly closed, and does not work when the system is shut down (Linux).

[PF4Public]

I don't see why not, you don't need our permission to make a PR!

It would make sense to first discuss the proposed changes before submitting a PR, it could potentially spare the effort if the proposed changes could complicate the maintenance later for example.

[benjamasu]

I want to try to implement the ability to add exceptions via site settings

It doesn't seem to be necessary. I didn't notice it right away, but such a feature is already implemented in Chromium. chrome://settings/content/siteData

I've considered that it's better not to introduce duplicate flags whose functions are already being fulfilled. So I decided it would be better to either remove #allow-clear-site-data-on-exit, or make it set an existing preference. I don't know how to implement the latter yet, but only have tracked this functionality down to class GeneratedCookieDefaultContentSettingPref.

[PF4Public]

@benjamasu Were you asking for help or just keeping us posted on your progress?

[fernvenue]

Any update here?