Focus-android: Investigate whether removing WebView files on erase is a good idea

[mcomella]

Related: #1569 Rethink erase strategy

---

After a browser session, WebKit leaves around some files. I filed Chromium bug [edit: the files that lay around in LocalStorage do so intermittently in Focus and never in a test project - I don't think this is a Chromium bug; I will continue to update]. We remove the files on erase after #433 (PR #476) as a preventative measure but it can create problems, like #1306:

deleting the "Local Storage" files appears to block the [ed. WebView] from deleting the localStorage data exposed to the web page (until the process is killed), which is why I suggest that we should, in the simplest case, leave the files when hitting erase but delete them at some later point [ed. when the process is dead]

This makes me question whether or not it is a good idea to be removing the files WebKit expects to be there because it may create these side-effects.

Notes:

• WebKit will only create its files the first time the WebView is used in this process: if you delete files, the WebView will not recreate them until the process is restarted. This implies it doesn't check for their existence and may perform unexpected behavior when they're missing
• If we don't delete the files, WebKit may leak history data to the filesystem. For example, in "Local Storage", the files are named after the domain they come from such as "support.google.com".

Guideline

As a guidline from https://github.com/mozilla-mobile/focus-android/issues/1306#issuecomment-336227143:

We say we do not store any history etc. - So I think our requirements are:

• Make sure that we do not accumulate traces of browsing history on disk. Temporary caches are okay.
• Do not leak any data to other apps.

I think as long as we stay in those bounds we can be flexible.

Approaches

I think some approaches are:

• Wait for Chromium to fix the issue
• Selectively fix issues as they come up (e.g. #1306: don't delete "Local Storage" files on erase)
• Find a comprehensive solution:
  
  
  
  • where we don't delete files on erase but clear them after the process ends
  
  
  • where we let WebKit let us delete the files. This is likely hacky. We might be able to do this by killing the Focus/Chrome (if it's separate) process, using ClassLoaders to unload the WebView classes, keeping a copy of its files in memory only and copying them to disk when we delete, etc.
  
  

In https://github.com/mozilla-mobile/focus-android/issues/1306#issuecomment-336226016 we began the discussion about some approaches we could take to not delete files on erase but clear them later:

• Deleting "Local Storage" contents when the process starts (in Application.onCreate; I confirmed this works)

That's what I was wondering too and I think we could try that.

• Deleting the "Local Storage" contents after the process ends (since there's no callback, we might have to schedule a repeating job in onStop)

This sounds hacky. I saw that a Service set to sticky will restart (and also the process!). Maybe we could use that to clean up and then shut it down again. But that's a bit hacky too.

---

Another note (for another bug?): @pocmo mentions we erase these files whenever a tab is removed (via back) and this seems extreme.

[mcomella]

After doing more work in #1306, I think we should also use this bug to rethink our erase strategy (make it a meta?). Notes:

• We have three kinds of erases: erases with a WebView instance, erases with static WebView methods, and erasing the files on disk. I'd suggest merging the last two.
• We call each of these three at different times for what seem like a lot of redundant calls, e.g. when sessions becomes empty (delete files on disk), onPause and finishing (delete files on disk), when the erase button is actually hit (all three)
• The methods are confusingly named, especially with the extra layers of abstraction like WebViewProvider

We should try to centralize the erase so it only happens in one place and perhaps when starting a completely fresh browsing session (since that's the workaround for #1306).

[mcomella]

After doing more work in #1306, I think we should also use this bug to rethink our erase strategy (make it a meta?).

Actually, that's called scope creep: let's go back to just deleting WebView contents. Note that the solution to both #1306 and #1492 was deleting less of the WebView files on disk.

[mcomella]

I think the action items for this bug, assuming we don't want to delete the files:

• Identify the files WebKit keeps on disk when we don't delete the directories
• Verify the files do not contain personal information
• For files with personal info:
  
  
  
  • See if there are APIs to clear them. If not...
  
  
  • Consider deleting them
  
  

Here's a list of the files WebKit keeps around in its directory after we erase:

/data/data/org.mozilla.focus.debug/app_webview # find .
.
./paks
./Web Data
./Web Data-journal
./Cookies
./Cookies-journal
./Local Storage
./GPUCache
./GPUCache/index
./GPUCache/index-dir
./GPUCache/index-dir/the-real-index
./databases
./databases/Databases.db
./databases/Databases.db-journal
./QuotaManager
./QuotaManager-journal

[liuche]

@mcomella will see what happens if just leaving these files, and report back to @bbinto.

[mcomella]

@bbinto Do you have specific STR for me to test here? (e.g. the steps from the store)

[bbinto]

@mcomella

Firefox Focus used on Samsung tablet, landscape mode

1. start page: http://shoppopbox.com/shopper/
2. Go through checkout (before hitting final submit) for e.g. Vinebox, you should see items in shopping card
3. Hit erase
4. Repeat steps 1 and 2 and you will see the items from the previous session in the card

@randyjensen, feel free to add more details. Thanks

[randyjensen]

A few more specific details if it helps:

1. The exact tablet being used.

2. We are using a Kiosk app called MobiLock which forces Firefox Focus to automatically open itself if the user hits the home button. I've tested with this disabled and it does not fix the issue.

3. We partner with online companies to sell their products in our store. The checkout experience is handled individually on that brand's website, so it's critical that all data gets wiped after a customer checks out. We actually lost a sale because a customer came in, went to check out, and saw the previous customer's CC, shipping, etc info from the last session.

4. I've compared FF Focus to Frost which appears to wipe everything correctly.

[mcomella]

@bbinto Do you have more specific steps to reproduce? I have been unable to reproduce with Vinebox and a few others but I've been hesitant to click through any CC forms because I'm concerned I'll actually order something. Some thoughts:

• For information to remain, I think either 1) the site needs to explicitly save the information for the user (for convenience), e.g. via Local Storage or 2) WebView caches <input>/form information
• If it's the former, local storage, I have an expected fix in 2.4 (which is our upcoming release) via #1306
• If not, WebView is likely caching the <input> info. To test this, I wanted to reproduce on master, comment out the code to erase the WebView files on disk, and then see if I could still reproduce.

@pocmo Do you think you can follow-up on this while I'm on vacation?

btw, thanks for the additional debug info, Randy! :)

[bbinto]

@mcomella -- could we send some of your proposed solutions as APK sizes to @randyjensen to try them out?

[randyjensen]

I'm doing more testing tomorrow to try and narrow things down as well. I'm in CA, not Chicago with the tablets, unfortunately.

I'm going to completely uninstall MobiLock to make certain that's not conflicting in any way. I'm also checking Samsung Knox to make sure it's disabled just in case. Will try to get as pure of a test as possible for you all.

A bit more background on why we're using MobiLock. Not only are we using it to make the tablet act like a Kiosk, it allows us to force Firefox Focus to re-open if the customer hits the home button on the tablet.

More importantly, it allows us to essentially set a "homepage" (our shopper page above) which Focus doesn't currently do. If we had a setting to set a default homepage, we could potentially get rid of MobiLock if it is actually causing more issues than I realize from my previous testing.

I can also test out some APKs if needed!

More soon!

[randyjensen]

OK, I had my team ship me an exact tablet that is having this issue. My earlier statement about this happening when our Kiosk software was running or not was only half correct.

After investigating more thoroughly, I've been able to consistently reproduce the bug and realized it may be out of scope since the issue is related more to MobiLock's settings, not Focus. I've still created a video in case this is something that affects another odd bug that may be lingering.

My test cases:

1. Focus running normally - Works properly
2. Focus running inside MobiLock - Works properly
3. Focus running inside MobiLock where Focus cannot be closed - Data Preserved

Reproducible steps (shown in this video):

1) Open MobiLock
2) Set MobiLock Settings to always open Firefox Focus on the tablet
3) Go to http://shoppopbox.com/shop
4) Click PupJoy logo
5) Click "Go!" blue button
6) Click "Add to cart" blue button
7) Product is in cart now, click the Clear Icon in Firefox Focus
8) Repeat steps 3-6. You'll see multiple items in the cart.

[bbinto]

Thanks @randyjensen -- it does sound like something we might not be able to help much/prioritize this at this point. Would using Focus inside Mobilock (with allowing it to be closed) an option for you in the meantime?

Also, I wonder if you can file a bug report with Mobilock for scenario 3.

[randyjensen]

We're going to test that out today and see how things go.

Getting in touch with the MobiLock team as we speak!

[mcomella]

I'm going to refocus this bug back on "Investigate whether removing WebKit files on erase is a good idea", now that Randy's issue seems to be unrelated.

---

Sebastian notes:

There's more data persisted (like service workers) that cannot be cleared via methods.

If true, we definitely have to erase Service Workers' data by hand.

[mcomella]

Susheel brought up a point in https://github.com/mozilla-mobile/focus-android/issues/1492#issuecomment-348371433 that paths may change across OEMs (and WebView versions) so we should consider this in our solution here!

[mcomella]

In investigating #1875, I changed the trash can to only:

CookieManager.getInstance().removeAllCookies(null);
deleteContentFromKnownLocations(getContext());

And confirmed that CookieManager still works, even after deleting the Cookie files on disk (which don't restore themselves after deletion).

[mcomella]

I noticed WebView stores a SharedPreferences file: https://cs.chromium.org/chromium/src/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java?q=lastVersionCodeused&sq=package:chromium&l=226 and that setHttpAuth doesn't seem to consistently clear data: https://issuetracker.google.com/issues/36936007

[bbinto]

Re-surfacing for triage after Focus release changes

[vesta0]

Moving to GeckoView.