[Webkit-unassigned] [Bug 134254] Don't allow for sudden termination while writing to local storage.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=134254


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #233742|review?                     |review+
               Flag|                            |




--- Comment #5 from Darin Adler <darin at apple.com>  2014-06-25 20:01:52 PST ---
(From update of attachment 233742)
View in context: https://bugs.webkit.org/attachment.cgi?id=233742&action=review

> Source/WebKit2/ChangeLog:3
> +        Don't allow for sudden termination while writing to local storage.

“Don’t allow sudden termination”

“Don’t allow for” means something different.

> Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:36
> +#include <WebCore/SuddenTermination.h>

Don’t need to add this both to the header and the cpp file. If you are unable to remove it from the header (see comment below), please remove it from here.

> Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.cpp:269
> +        if (m_disableSuddenTerminationWhileWritingToLocalStorage)
> +            m_disableSuddenTerminationWhileWritingToLocalStorage.reset();

Normally we just assign nullptr instead of using the reset function. Also, it’s overkill to add the if statement. So these two lines should be replaced with just this:

    m_disableSuddenTerminationWhileWritingToLocalStorage = nullptr;

Is there something that guarantees the database is flushed out to disk? How did you test?

> Source/WebKit2/UIProcess/Storage/LocalStorageDatabase.h:30
> +#include <WebCore/SuddenTermination.h>

Do we really need this include? Can we just do a forward declaration instead?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.