Adds preferences and methods to delete Leo conversations on Android #27074

SergeyZhukovsky · 2024-12-19T19:54:23Z

Issues:

Resolves Android Settings can delete Leo AI history brave-browser#42995

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

STR 1:

Make some chats using Leo AI.
Go to Settings->Leo->Delete all Leo AI conversation data and click confirm.
Make sure all chats history is deleted.

STR 2:

Make some chats using Leo AI.
Go to Settings->Brave Shields & privacy->Delete browsing data and go to Advanced tab there.
Check Leo AI and click Delete data and confirm it.
Make sure all chats history is deleted.

STR 3(make sure that deleting browsing history deletes any related web page visit from conversations):

Go to brave.com.
Open Leo tab and summarize the page.
Make sure you see the website info if you open the summarized conversation from the conversation list.
Close Leo tab.
Go to Settings->Brave Shields & privacy->Delete browsing data and go to Basic tab there. Make sure Browsing history is checked there
Go to a new tab and open Leo tab from there.
Select the summarized conversation from the conversation list and make sure there is no website info

SergeyZhukovsky · 2024-12-19T19:56:07Z

@diracdeltas let me know do you want me to open an issue for a sec review. I didn't do it as it's basically a fix related to the original sec review for that feature.

samartnik

java patching lgtm

android/java/res/layout/brave_leo_clear_history_dialog.xml

petemill · 2024-12-19T20:06:29Z

browser/ai_chat/ai_chat_settings_helper.cc

+  if (!service) {
+    return;
+  }
+  service->DeleteConversations();


I wonder if we just pass the AIChatService mojo to the UI instead of passing it via the AIChatSettingsHelper mojo?

I don't think we ever passed native services to java via mojo. Why do we need that?

we are passing AIChatSettingsHelper to Java and its calling a function which then calls the service function only. So why not just pass the Service to Java now that we have one? AIChatSettingsHelper was made before AIChatService and now that it exists maybe we shouldn't add new things to AIChatSettingsHelper

as per discussion in DM we decided to leave as is as there are questions about the service lifetime, there has to be a native object that hold a pointer for Java.

AlexeyBarabash

lgtm

deeppandya

LGTM

diracdeltas · 2024-12-19T20:39:30Z

Shivan is out, but the original privacy requirement from https://github.com/brave/reviews/issues/1768#issuecomment-2536586219 said Regular browser history data deletion deletes any related web page visit from conversations.

Is that the case with this PR? If so there should be a test plan that clearing browser history also clears leo or at least any leo conversations with page context. cc @bridiver if this is your understanding since you were involved in the above privacy review.

This is also stated in @petemill 's comment here btw https://github.com/brave/reviews/issues/1703#issuecomment-2523752623. The items below should be implemented on Android before this feature is enabled.

A Notice that encrypted conversation persistance is now enabled is shown to all users (with link to the help center article which explains how to disable it)
There is a setting in Brave's Preferences which disables storage
Regular browser history data deletion deletes any related web page visit from conversations
A new "Leo AI" browser data deletion checkbox deletes all AI conversations

petemill · 2024-12-19T21:10:07Z

Shivan is out, but the original privacy requirement from brave/reviews#1768 (comment) said Regular browser history data deletion deletes any related web page visit from conversations.

Is that the case with this PR? If so there should be a test plan that clearing browser history also clears leo or at least any leo conversations with page context. cc @bridiver if this is your understanding since you were involved in the above privacy review.

This is also stated in @petemill 's comment here btw brave/reviews#1703 (comment). The items below should be implemented on Android before this feature is enabled.

A Notice that encrypted conversation persistance is now enabled is shown to all users (with link to the help center article which explains how to disable it)
There is a setting in Brave's Preferences which disables storage
Regular browser history data deletion deletes any related web page visit from conversations
A new "Leo AI" browser data deletion checkbox deletes all AI conversations

I think Android uses the same browser data deletion handling as desktop @diracdeltas

SergeyZhukovsky · 2024-12-19T21:21:35Z

Is that the case with this PR? If so there should be a test plan that clearing browser history also clears leo or at least any leo conversations with page context.

@diracdeltas correct it is the case with this PR. I just added STR3 with that case in the test plan.

petemill · 2024-12-19T21:58:45Z

...d/java/org/chromium/chrome/browser/browsing_data/BraveClearBrowsingDataFragmentAdvanced.java

@@ -22,17 +23,30 @@
 import org.chromium.ui.text.SpanApplier.SpanInfo;

 public class BraveClearBrowsingDataFragmentAdvanced extends ClearBrowsingDataFragmentAdvanced {
+    ClearBrowsingDataCheckBoxPreference mClearBrowsingDataCheckBoxPreference;


Should this be named to mention that it's the checkbox for AIChat? e.g. mClearAIChatDataCheckBoxPreference?

good point, renaming it

petemill · 2024-12-19T22:00:26Z

...d/java/org/chromium/chrome/browser/browsing_data/BraveClearBrowsingDataFragmentAdvanced.java

+    @Override
+    protected void onClearBrowsingData() {
+        super.onClearBrowsingData();
+        if (mClearBrowsingDataCheckBoxPreference != null
+                && mClearBrowsingDataCheckBoxPreference.isChecked()) {
+            Profile profile = getProfile();
+            if (profile == null) {
+                return;
+            }
+            BraveLeoMojomHelper.getInstance(profile).deleteConversations();
+        }
+    }


Do we need this? Hopefully it gets handled by the shared browser code: https://github.com/brave/brave-core/blob/master/browser/browsing_data/brave_browsing_data_remover_delegate.cc#L55-L79

In Desktop's settings WebUI we add a new checkbox with a browser.clear_data.brave_leo key (should have been ai_chat!). The desktop WebUI (and assume somewhere in android preferences) we call browsing_data::GetDataTypeFromDeletionPreference which we override to provide BrowsingDataType::BRAVE_AI_CHAT when the browser.clear_data.brave_leo key is provided. And the rest is handled by the delegate.

we basically discussed that in DM it gets triggered on Android, but for Android case we would need to make a lot of patches inside java files by adding extra type inside ClearBrowsingDataFragment.DialogOption and surrounding
functions. I added a comment inside browser/browsing_data/brave_browsing_data_remover_delegate.cc where Android implementation is.

bsclifton

string reviewers ++

aseren

LGTM

brave-builds · 2024-12-23T00:03:34Z

Released in v1.75.117

SergeyZhukovsky requested review from diracdeltas, petemill, simoarpe, fallaciousreasoning, AlexeyBarabash and aseren December 19, 2024 19:54

SergeyZhukovsky self-assigned this Dec 19, 2024

SergeyZhukovsky requested review from deeppandya and a team as code owners December 19, 2024 19:54

samartnik approved these changes Dec 19, 2024

View reviewed changes

SergeyZhukovsky commented Dec 19, 2024

View reviewed changes

android/java/res/layout/brave_leo_clear_history_dialog.xml Show resolved Hide resolved

petemill reviewed Dec 19, 2024

View reviewed changes

AlexeyBarabash approved these changes Dec 19, 2024

View reviewed changes

deeppandya approved these changes Dec 19, 2024

View reviewed changes

This was referenced Dec 19, 2024

AI Chat history feature should be disabled by default on Android brave/brave-browser#42928

Closed

Android Settings can delete Leo AI history brave/brave-browser#42995

Closed

petemill reviewed Dec 19, 2024

View reviewed changes

Adds preferences and methods to delete Leo conversations on Android

c181adf

SergeyZhukovsky force-pushed the android_delete_leo_history branch from 3df3911 to 10b350a Compare December 20, 2024 16:25

Adds time period for Leo conversation delete functionality on Android

4d05ccd

SergeyZhukovsky force-pushed the android_delete_leo_history branch from 10b350a to 4d05ccd Compare December 20, 2024 16:46

SergeyZhukovsky requested a review from a team December 20, 2024 19:41

bsclifton approved these changes Dec 20, 2024

View reviewed changes

aseren approved these changes Dec 20, 2024

View reviewed changes

diracdeltas approved these changes Dec 20, 2024

View reviewed changes

petemill approved these changes Dec 20, 2024

View reviewed changes

SergeyZhukovsky merged commit 9f3121e into master Dec 20, 2024
18 checks passed

SergeyZhukovsky deleted the android_delete_leo_history branch December 20, 2024 23:52

github-actions bot added this to the 1.75.x - Nightly milestone Dec 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds preferences and methods to delete Leo conversations on Android #27074

Adds preferences and methods to delete Leo conversations on Android #27074

SergeyZhukovsky commented Dec 19, 2024 •

edited by petemill

Loading

SergeyZhukovsky commented Dec 19, 2024

samartnik left a comment

petemill Dec 19, 2024

SergeyZhukovsky Dec 19, 2024

petemill Dec 19, 2024

SergeyZhukovsky Dec 20, 2024

AlexeyBarabash left a comment

deeppandya left a comment

diracdeltas commented Dec 19, 2024 •

edited

Loading

petemill commented Dec 19, 2024

SergeyZhukovsky commented Dec 19, 2024 •

edited

Loading

petemill Dec 19, 2024

SergeyZhukovsky Dec 20, 2024

petemill Dec 19, 2024

petemill Dec 19, 2024

SergeyZhukovsky Dec 20, 2024

bsclifton left a comment

aseren left a comment

brave-builds commented Dec 23, 2024

Adds preferences and methods to delete Leo conversations on Android #27074

Adds preferences and methods to delete Leo conversations on Android #27074

Conversation

SergeyZhukovsky commented Dec 19, 2024 • edited by petemill Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

SergeyZhukovsky commented Dec 19, 2024

samartnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexeyBarabash left a comment

Choose a reason for hiding this comment

deeppandya left a comment

Choose a reason for hiding this comment

diracdeltas commented Dec 19, 2024 • edited Loading

petemill commented Dec 19, 2024

SergeyZhukovsky commented Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsclifton left a comment

Choose a reason for hiding this comment

aseren left a comment

Choose a reason for hiding this comment

brave-builds commented Dec 23, 2024

SergeyZhukovsky commented Dec 19, 2024 •

edited by petemill

Loading

diracdeltas commented Dec 19, 2024 •

edited

Loading

SergeyZhukovsky commented Dec 19, 2024 •

edited

Loading