Bug 108733 - WebCore Blob refactoring meta bug.
Summary: WebCore Blob refactoring meta bug.
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Nordman
Depends on: 108736 108851
  Show dependency treegraph
Reported: 2013-02-01 17:54 PST by Michael Nordman
Modified: 2013-04-17 12:14 PDT (History)
3 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Michael Nordman 2013-02-01 17:54:28 PST
An umbrella bug for a series of changes to hopefully improve upon WebCore::Blob handling. I've been working on a set of changes for a while locally and am to the point of wanting to land changes. Some notes that hopefully give a decent overview.

For where I'd like to get to, you can look at these chromium code review tool uploads.
chromium: https://codereview.chromium.org/11410019/
webkit: https://codereview.chromium.org/11192017/
tests: https://codereview.chromium.org/11416382/

Chrome SVN changes

- Added BlobStorageContext as a replacement for BlobStorageController. The new class manages blob data by uuid and seperately maintains a mapping from of public blob urls to uuids.
- Added BlobStorageConsumer class, a container for blob uuid and public url usage associated with a renderer/worker process. Cleans up on exit.
- Added BlobDataHandle. In-browser-process scoper object for a reference to a blob.
- Changed blob processing classes to take BlobDataHandles as input instead of GURLs. Not long after IPC deserizliation, get a BlobDataHandle and pass that around.
    - FileSystemOperation.Write (and famlity) for FileWriter support
    - ResourceDispatcherHost (and famility) for blob url requests
    - (PostMessage handling and WebIntent plumbing ultimately needs to be treated in this way too)
- Added OnDidCreateSnapshotFile callback path, no longer overloading OnDidReadMetadata for that purpose.
- Removed dependency on blobs from the browser-side of SnapshotFile creation.
- Switched to using string uuids instead of urls in IPC messages (and structs carried in ipc messages)
   - FileSystemHostMsg_Write
   - webkit_base::DataElement (and consumers: BlobData, ResourceRequestBody)
   - blob registry/building ipc messages
- Implemented the WebKit::WebBlobRegistryImpl such that it can be invoked on any renderer/worker thread.
- Got more explicit about naming: FileSystemURL vs BlobUUID vs PublicBlobURL.

WebKit/WebCore SVN changes

- Added BlobDataHandle. Renderer/worker side scoper object for a reference to a blob.
- Change blob processing/handling classes to work with BlobDataHandles insead of URLs.
    - WebCore::Blob
    - WebCore::BlobBuilder
    - postMessage() plumbing
    - WebSocket.send() plumbing
    - fileWriter.write() plumbing
    - SerializedScriptValue
    - FormData
- Switched to using string uuids instead of urls in structs and params in the WebKit API.
      - WebBlobData::Item
      - WebHTTPBody::Element
      - WebFileWriter::write()
      - WebBlobRegistry
- Defined/implemented WebCore::BlobRegistry to be callable from any renderer/worker thread.
   - BlobRegistryImpl for 'native' webcore
   - BlobRegistryProxy for chrome
- Removed ThreadableBlobRegistry.
- Added didCreateSnapshotFile plumbing that carries a WebCore::BlobDataHandle with it thru the bridging/callback/wrapper forest.
- Got more explicit about naming: FileSystemURL vs BlobUUID vs PublicBlobURL.
Comment 1 Michael Nordman 2013-02-01 17:56:21 PST
I'm still not sure how to sequence the landing of these changes between chromium and webkit/webcore. I'm starting off by pulling out the modifications to how FileSystem::createSnapshot.
Comment 2 Michael Nordman 2013-02-04 15:57:12 PST
I've opened a related tracking bug in on crbug.com.
Comment 3 Michael Nordman 2013-03-07 16:28:36 PST
I think I see how to go about the two-sided landing dance on this. Roughly speaking...

Land the chromium side first with additions that allow us to continue to support the old URL-per-WebCore::Blob identifiers.

That's mostly a matter of providing the existing Registry interface in terms of the new backend in chromium.
  void registerBlob(url, data)
  void registryBlob(newurl, existingurl)
  void unregisterBlob(url)

For the first coin a uuid and register <uuid,data> in the new backend, and additionally estabishing registerOldStyle(url, uuid) that creates a mapping from that url to the uuid in the new backend. The others are similar in that they affect that mapping.

Its also a matter of continuing to take bloburls as inputs in a couple of other WebKit interfaces: WebFileWriter.write() and WebHTTPBody::Element and WebBlobData::Item. When processing those inputs on the browser process, I'll need to get a chromium BlobDataHandle given the url which is simple enough to do given that mapping setup above.

I'll update the chromium-side changes I've got along those lines. At that point, I should be able to run tryjobs and tests. Once landed in chromium, I'll come back to the webkit/webcore side.
Comment 4 Michael Nordman 2013-04-17 12:14:13 PDT
I'm closing this bug out as it was work to better support chromium most directly and given Blink I'm not planning to proceed with these changes here and the bug is assigned to me.

Although aspects of the changes described may still be relevant/desired for other webcore consumers. If so, please reopen and reassign.