Favicon DDOS

[luixxiul]

Describe the issue you encountered:
As growing the number of users, there will be a possibility of starting DDOS to websites after it becomes possible to add them manually on about:preferences#search as long as favicons are loaded from them directly. This might be critical for small sites.

This also applies to favicons on bookmarks bar, about:bookmarks page, etc.

Expected behavior:
I think there are some ways: (1) stop displaying favicons completely, (2) use local cache, (3) fetch favicons only if the site is not in the history, (4) don't let users add sites . I think 2 or 3 is acceptable here.

• Platform (Win7, 8, 10? macOS? Linux distro?): Windows 10
• Brave Version: 0.11.2 Beta2
• Any related issues: Cache favicons (to prevent fetching each time Brave is reloaded) #2697

[cndouglas]

I would prefer 2, which is similar to how Firefox does it. The current behavior, retrieving the favicon every time a new window is opened, seems rather wasteful.

[luixxiul]

This might be an issue if a lot of people start clearing cached files on closing the browser with private data options.

[luixxiul]

I think that favicons on bookmarks toolbar are actually fetched at every restart, bypassing local cache (if any). Here are steps to reproduce the issue.

1. Bookmark ~10 websites with favicons
2. Hide the bookmark toolbar
3. Restart the browser
4. Open Task Manager of Windows
5. Show the bookmark toolbar
6. Look at the network column of the task manager

I noticed ~1.0 Mbps bandwidth at most was consumed on showing the bookmark toolbar with ~20 favicons.

[luixxiul]

I am not sure if this is reproducible on Mac too.

[cndouglas]

It is reproducible on macOS. Favicons are downloaded on launch.

[bsclifton]

For content pages like about:bookmarks, images are cached. However, images that are in the browser (ex: favicons in the bookmarks toolbar + the ones from the back/forward nav buttons) do not seem to be caching. When discussing with @bridiver, it seems that these are not using a public context when fetching, so it doesn't get cached

[bsclifton]

I'm going to close this issue in favor of #2697

(I had some notes here, will post there instead)