New top-level storage directory

57 views
Skip to first unread message

pil...@chromium.org

unread,
Aug 6, 2014, 5:30:29 PM8/6/14
to stora...@chromium.org, John Abd-El-Malek
(My apologies if this is the wrong list. It appeared to have most of the people I would have emailed about this individually, and it has the advantage of being public.)

I've been working with a few other Chromium developers to refactor the webkit/ directory. Most of it has gone into content/ or content/public/, but what remains is a lot of inter-related storage classes (fileapi, database, quota, blob, data_element). In talking with Darin Fisher about it, we came up with a possible plan: a new top-level storage/ directory.

Key points:

- Both content/ and chrome/ would depend on storage/. (This is the same as webkit/ now.)
- storage/ would not have dependencies on content/ or chrome/. (This is the same as webkit/ now.)
- storage/ would depend on base/, net/, sql/, third_party/WebKit/public/platform/, url/, third_party/leveldatabase/src/include/leveldb/, and third_party/sqlite/. (This is the same as webkit/ now.)
- storage would have 1 gyp/gn file (this is fewer than webkit/ has now)
- storage would have 2 build targets (storage_common and storage_browser) (this is 1 fewer than webkit/ has now)
- storage would have 2 export macros (STORAGE_EXPORT and STORAGE_EXPORT_PRIVATE) (this is fewer than webkit/ has now)

Thoughts? Feedback? Concerns?

Yours in refactoring,
-Mark

Michael Nordman

unread,
Aug 7, 2014, 7:53:56 PM8/7/14
to pil...@chromium.org, stora...@chromium.org, John Abd-El-Malek
Seems like database, quota, blob (and data_element) would be more or less straightforward to move into /content but fileapi is the real sticking point... and since it depends on quota and blob... they're stuck too. Is that right? If that is right, at least database probably should continue on its way into content.

I'm looking at https://codereview.chromium.org/442383002/ to get an idea of the dependencies on fileapi in /chrome and /extensions. There are a great many in cros (filemanager and gdrive) and in chrome (syncfilesystem and mediagalleries). Yikes. I understand the dependencies on the abstract FileSystemOperation class (this is what gdrive and syncfs have to implement)  and the concrete FileSystemURL class.

I'd like to better understand what /chrome depends on out of fileapi.

If we do the new top-level-dir, instead of calling the new top-level lib /storage, might make sense to call it /fileapi since most of the storage components have already migrated to /content.

Kinuko Yasuda

unread,
Aug 8, 2014, 11:45:28 AM8/8/14
to Michael Nordman, pil...@chromium.org, stora...@chromium.org, John Abd-El-Malek
On Fri, Aug 8, 2014 at 8:53 AM, Michael Nordman <mich...@chromium.org> wrote:
Seems like database, quota, blob (and data_element) would be more or less straightforward to move into /content but fileapi is the real sticking point... and since it depends on quota and blob... they're stuck too. Is that right? If that is right, at least database probably should continue on its way into content.

I'm looking at https://codereview.chromium.org/442383002/ to get an idea of the dependencies on fileapi in /chrome and /extensions. There are a great many in cros (filemanager and gdrive) and in chrome (syncfilesystem and mediagalleries). Yikes. I understand the dependencies on the abstract FileSystemOperation class (this is what gdrive and syncfs have to implement)  and the concrete FileSystemURL class.

I'd like to better understand what /chrome depends on out of fileapi.

Fyi, here's what I was maintaining for cleaning up webkit fileapi/ code while I was working on the similar refactoring (though never finished doing so):

It's been while and looks like the # of fileapi header files that are referred in chrome/ has increased a bit since then (18 files -> 25 files, per a quick git grep)

If we do the new top-level-dir, instead of calling the new top-level lib /storage, might make sense to call it /fileapi since most of the storage components have already migrated to /content.

I was actually thinking about it after some failed attempts to cleanly cut public content/ interface for fileapi.  It defines several interfaces that are implemented in chrome/ (e.g. file operation, file reader, file writer, mount point), and exporting all those interfaces from content/ would make the public content API bloated.  If it could be put in its own top-level directory the cleanup would become a bit easier.

(Btw thanks for moving this cleanup forward)

Darin Fisher

unread,
Aug 8, 2014, 4:05:39 PM8/8/14
to Kinuko Yasuda, Michael Nordman, pil...@chromium.org, stora...@chromium.org, John Abd-El-Malek
It occurs to me that another reason to favor a new top-level directory here is to support better modularization. We are working toward a world where browser-side services are exposes to the renderer (or other sandboxed processes) via Mojo IPC. In that world, it would be nice to have such services less coupled to content/. For this reason, going with src/storage/ might make sense even though it would at first only contain File API related stuff.

-Darin

Michael Nordman

unread,
Aug 8, 2014, 4:50:13 PM8/8/14
to Darin Fisher, Kinuko Yasuda, pil...@chromium.org, stora...@chromium.org, John Abd-El-Malek
honestly, it always pained me a little to see our somewhat modularized storage libs get squished directly into the content, although the act of doing so has served as a forcing function to create limited public apis for use by /chrome that's distinct from the rich api's used by /content.

should there be different DEPs rules for /content and /chrome on the /storage lib such that /content is free to use more than /chrome?

Darin Fisher

unread,
Aug 8, 2014, 5:03:40 PM8/8/14
to Michael Nordman, pil...@chromium.org, John Abd-El-Malek, stora...@chromium.org, Kinuko Yasuda

Yeah, pros and cons. Probably could have API tiers. Hmm...

Reply all
Reply to author
Forward
0 new messages