Skip to content

Add recents, offlinepage and send to home screen for always incognito#1427

Merged
csagan5 merged 2 commits into
bromite:masterfrom
uazo:history-in-incognito-2
Oct 19, 2021
Merged

Add recents, offlinepage and send to home screen for always incognito#1427
csagan5 merged 2 commits into
bromite:masterfrom
uazo:history-in-incognito-2

Conversation

@uazo
Copy link
Copy Markdown
Collaborator

@uazo uazo commented Oct 2, 2021

here is the patch, unfortunately a bit big. what's up:

Add to homescreen in always incognito

it was enough to act in AppMenuPropertiesDelegateImpl.java to show the button and enable isAddToHomeIntentSupported.
Note that I only enabled it for always incognito because the open from the homescreen icon is always in primary mode (in normal navigation, so the homepage sites saved incognito are reopened not in incognito, and that's not good).

Enable recent tabs menu in always incognito

it was sufficient to act in AppMenuPropertiesDelegateImpl.java to show the button and enable the registration of the tab in TabPersistentStore.java at the moment of closing. in RecentTabsManager I simply force the use of the primary profile, simply because otherwise the list of closed tabs would not be persisted. The modification in HistoricalTabSaver is simply to bring me the information of the incognito mode between java and native (the always incognito flag is only on the java side).
Note that this feature is also active only for always incognito because reopening from the page is always in primary mode.

Save page in (always) incognito

A little more complex, first some technical notes (I hope not to write nonsense and not to offend anyone): first, it is not a simple creation of an mhtml file as I thought.
the offline mode is managed by OfflinePageModelFactory which derives from SimpleKeyedServiceFactory, which is chromium's way of managing dependency on services. The services are connected to BrowserContext, in our case by its derivation based on the profiles (ProfileKey) from which the current profile (normal or offtherecord, i.e. incognito) and where the preferences service can be retrieved. Side note: the profiles themselves are an extension of the BrowserContext and that from the incognito profiles you can retrieve the normal profile via GetOriginalProfile(), useful for having the real persisted preferences.

So, the work was to re-enable OfflinePageModelFactory and everything needed for his management, and I found RequestCoordinatorFactory which I re-enabled for incognito management.

The final step is to find where the saved pages are reopened, that is in the bridge between java and native (OfflinePageBridge): here I forced the page to open as if it were a normal mhtml file opened by the user. I preferred this way because alternatively I would have had to rehabilitate other services and the relationship between them, which do not seem to be necessary.

As I said here, too, I preferred activation only with always incognito: I do not have the ability to predict whether now or future functionalities can allow the passage of information between normal navigation and incognito (without always incognito active).
so for this I do not link any issues, where the request were different

The changes instead you see in HistoryPage and HistoryManager are for v94 where the new IncognitoPlaceholder does not allow viewing the history in incognito. In v94 it is active by default, in v93 seem to be under a flag

Tests

a note on tests: I'm starting to insert the tests in the patches that I do and that you find in this one too, but the question is a bit complex and must be explained separately.

Comment thread build/patches/Add-history-support-in-incognito-mode.patch
@csagan5
Copy link
Copy Markdown
Contributor

csagan5 commented Oct 14, 2021

I simply force the use of the primary profile, simply because otherwise the list of closed tabs would not be persisted

Will not this breach the principle of not storing outside of RAM incognito navigation details?

As I said here, too, I preferred activation only with always incognito: I do not have the ability to predict whether now or future functionalities can allow the passage of information between normal navigation and incognito (without always incognito active).
so for this I do not link any issues, where the request were different

Does it mean that the recent tabs feature requires the history in incognito to be enabled? Or, put the other way round: if history in incognito is disabled, will the recent tabs show in the history once someone enables that history in incognito setting?

@uazo
Copy link
Copy Markdown
Collaborator Author

uazo commented Oct 14, 2021

Will not this breach the principle of not storing outside of RAM incognito navigation details?

we already do, but only in always incognito with history enabled

Does it mean that the recent tabs feature requires the history in incognito to be enabled?

tecnically yes, recents tabs needs some internal history services, in incognito are not enabled, even with history flag enabled.
i've activated that only in always incognito + history enabled

Or, put the other way round: if history in incognito is disabled, will the recent tabs show in the history once someone enables that history in incognito setting?

no, recents tabs do not read the history, they only record the closing of the tab, if nothing is active, nothing is recorded.

my patch activates that only with always incognito + history flag, I have not touched the code related to normal incognito mode, so even with history enabled recents tabs are not shown or recorded

@uazo
Copy link
Copy Markdown
Collaborator Author

uazo commented Oct 15, 2021

tecnically yes, recents tabs needs some internal history services, in incognito are not enabled, even with history flag enabled.
i've activated that only in always incognito + history enabled

rereading it, I was not precise (I will not answer again in the evening! :)

the reason why I did not activate it is for what I wrote, the opening is always in the primary mode, ie the normal one if without always incognito. basically the recents tabs work with the primary profile because they create an sqllite db in the primary partition (read, directory).
incognito mode uses a fake partition, that is not on disk but only in memory: to make it active in that mode I would have to heavily modify the code, managing a new sqllite db in memory (as does the db of cookies, which they call CookieMonster), too invasive.

@csagan5
Copy link
Copy Markdown
Contributor

csagan5 commented Oct 15, 2021

Ok, so if I understand correctly once this patch is merged if the user has always-incognito AND always-incognito history, then they get the recent tabs feature working?

@uazo
Copy link
Copy Markdown
Collaborator Author

uazo commented Oct 16, 2021

if the user has always-incognito AND always-incognito history, then they get the recent tabs feature working?

yes, that's right, with offline pages

@csagan5
Copy link
Copy Markdown
Contributor

csagan5 commented Oct 16, 2021

Ok, I see; can you expand the patch description to mention more of this information (also from PR description)?

So that it is not lost.

@csagan5 csagan5 merged commit e6c854c into bromite:master Oct 19, 2021
calyxos-gerrit pushed a commit to CalyxOS/chromium that referenced this pull request Mar 8, 2023
Add a preference that causes all new tabs and all clicked links to launch in incognito.
Make sure initial incognito status is correctly recognized.
Enable incognito custom tabs and fix crashes for incognito/custom tab intents
Use a native flag to correctly start new tabs on app startup

Add history, recents, offlinepages and send to home screen support for always incognito.
History, recent tabs and offline pages require the INCOGNITO_TAB_HISTORY_ENABLED
flag turned on.
IncognitoPlaceholder is also deactivated, both in the phone and tablet version.
The relative tests are also present.

based on the original work by Ryan Archer <ryan.bradley.archer@gmail.com>
Major contributions by uazo.
See also: bromite/bromite#1427

License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html
@uazo uazo deleted the history-in-incognito-2 branch July 18, 2023 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants