[webkit-reviews] review granted: [Bug 235897] Write origin file when OriginStorageManager is destroyed : [Attachment 450427] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 235897: Write origin file when OriginStorageManager is destroyed
https://bugs.webkit.org/show_bug.cgi?id=235897

Attachment 450427: Patch

https://bugs.webkit.org/attachment.cgi?id=450427&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 450427
  --> https://bugs.webkit.org/attachment.cgi?id=450427
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450427&action=review

> Source/WebKit/ChangeLog:12
> +	   The first WebStorage message sent from web process to network
process is sync, and writing origin file when 
> +	   creating OriginStorageManager will delay the reply. Since we can get
the origin information from in-memory map 
> +	   when OriginStorageManager is present, we may delay the write to
until when OriginStorageManager is destroyed.
> +	   This fixes the PLT regression from r286936.

Can we make a regression test for this?

Does it matter if this data gets lost because of a crash?

> Source/WebKit/NetworkProcess/storage/OriginStorageManager.cpp:283
> +    if (m_writeOriginFileFunction)
> +	   m_writeOriginFileFunction();

It’s often a poor pattern to do side effects like writing to a file in a
destructor or constructor. In this case it may be a helpful pattern, but I am a
bit concerned that call sites that are removing an element from a map don’t
look like they are doing file I/O.