Bug 224092 – Introduce ScriptBuffer class to wrap SharedBuffer containing a script

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[Chris Dumez]

Introduce ScriptBuffer class to SharedBuffer containing a script. We started using SharedBuffer to represent worker scripts instead of String, so that they can hold file mapped data and be shared across processes.
This patch introduces a new ScriptBuffer to wrap those SharedBuffers. The type makes it clearer what type of data we're dealing with. The helper functions used to convert between String and SharedBuffer can now simply be member functions on ScriptBuffer.
This also simplifies IPC code.

[Chris Dumez]

Created attachment 424983 [details]
Patch

[Chris Dumez]

Created attachment 424984 [details]
Patch

[Geoffrey Garen]

Can we use ScriptBuffer for regular scripts in the HTTP cache too?

[Chris Dumez]

(In reply to Geoffrey Garen from comment #3)
> Can we use ScriptBuffer for regular scripts in the HTTP cache too?

The HTTP cache serves many different types of files, not just scripts. I don't think it would be practical to use it there. The HTTP cache uses ShareableResource::Handle directly.

[Yusuke Suzuki]

Comment on attachment 424984 [details]
Patch

r=me

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 424984 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Geoffrey Garen]

r=me

It's nice to get this improvement for workers, but it really makes me wonder what we do for other scripts, and if we can share some code.

[Chris Dumez]

Created attachment 425050 [details]
Patch

[Chris Dumez]

(In reply to Geoffrey Garen from comment #7)
> r=me
> 
> It's nice to get this improvement for workers, but it really makes me wonder
> what we do for other scripts, and if we can share some code.

Main thread scripts are not really treated different from other resources. They are stored in the memory cache as a SharedBuffer inside the CachedResource. Once the network process has saved the resource to disk, it sends a URL + ShareableResource::Handle to the WebProcess. The WebProcess looks up the CachedResource using the URL in the memory cache, then tries and replace the CachedResource's SharedBuffer with the file-mapped version.

This logic does not currently work with dedicated or service workers because they do not use CachedScript or the memory cache. The memory cache is currently main-thread only.

Also, service workers are interestingly different (than usual network resources or even dedicated worker scripts) because they are persisted on disk with the service worker registration, independently from the HTTP disk cache. Therefore, it is highly preferable to use a version of the service worker script file-mapped from the service worker storage on disk, rather than the HTTP disk cache.

For dedicated workers, we should in theory be able to leverage the existing NetworkProcessConnection::DidCacheResource IPC to replace the in-memory version of the script with the file-mapped version from the HTTP disk cache. I have not worked on this part yet and I have been focusing on Service Workers since this is what's covered by PLUM3.

[EWS]

Committed r275443: <https://commits.webkit.org/r275443>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425050 [details].

[Radar WebKit Bug Importer]

<rdar://problem/76166419>

[Geoffrey Garen]

> They are stored in the memory cache as a SharedBuffer inside the
> CachedResource. Once the network process has saved the resource to disk, it
> sends a URL + ShareableResource::Handle to the WebProcess. The WebProcess
> looks up the CachedResource using the URL in the memory cache, then tries
> and replace the CachedResource's SharedBuffer with the file-mapped version.

I see: CachedScript has a dirty string m_script; but that's the decoded data that we throw away periodically. The encoded data is already SharedBuffer.