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

SergeyZhukovsky · 2024-12-19T20:00:25Z

android/java/res/layout/brave_leo_clear_history_dialog.xml

@@ -4,8 +4,14 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at https://mozilla.org/MPL/2.0/. -->

-<TextView xmlns:android="http://schemas.android.com/apk/res/android"
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"


@deeppandya we need LinearLayout in that case as we need to apply AlertDialogContent styles to the dialog.

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

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?

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.

Adds preferences and methods to delete Leo conversations on Android

3df3911

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

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

Open

petemill reviewed Dec 19, 2024

View reviewed changes

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

SergeyZhukovsky Dec 19, 2024

petemill Dec 19, 2024

SergeyZhukovsky Dec 19, 2024

petemill Dec 19, 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

petemill Dec 19, 2024

petemill Dec 19, 2024

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

Are you sure you want to change the base?

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

SergeyZhukovsky Dec 19, 2024

Choose a reason for hiding this comment

petemill Dec 19, 2024

Choose a reason for hiding this comment

SergeyZhukovsky Dec 19, 2024

Choose a reason for hiding this comment

petemill Dec 19, 2024

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

petemill Dec 19, 2024

Choose a reason for hiding this comment

petemill Dec 19, 2024

Choose a reason for hiding this comment

petemill Dec 19, 2024

Choose a reason for hiding this comment

SergeyZhukovsky commented Dec 19, 2024 •

edited by petemill

Loading

diracdeltas commented Dec 19, 2024 •

edited

Loading

SergeyZhukovsky commented Dec 19, 2024 •

edited

Loading