Bug 30602 - [V8] DOM Storage runtime flag changes
Summary: [V8] DOM Storage runtime flag changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-20 15:01 PDT by Jeremy Orlow
Modified: 2009-11-09 16:49 PST (History)
4 users (show)

See Also:


Attachments
Patch v1 (7.53 KB, patch)
2009-10-20 15:07 PDT, Jeremy Orlow
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2009-10-20 15:01:41 PDT
Revert my changes to Settings and instead implement DOM Storage enabling via the methods agreed upon in https://bugs.webkit.org/show_bug.cgi?id=30240
Comment 1 Jeremy Orlow 2009-10-20 15:07:30 PDT
Created attachment 41525 [details]
Patch v1
Comment 2 Adam Barth 2009-10-20 15:16:02 PDT
Comment on attachment 41525 [details]
Patch v1

Wow.  That's super cool.
Comment 3 David Levin 2009-10-20 15:18:19 PDT
Hmm. Adam reviewed at the same time....

Here were my comments:

Two minor nits to fix on landing.

> Index: WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
>  #include "ScriptSourceCode.h"
>  #include "SerializedScriptValue.h"
>  #include "Settings.h"
> +#include "Storage.h"
>  #include "SharedWorkerRepository.h"

"Storage" should be after "SharedWorker..."


> Index: WebCore/storage/Storage.cpp
> +#if USE(V8)
> +// TODO: Remove once these features are turned on by default in Chromium.

s/TODO/FIXME/
Comment 4 Jeremy Orlow 2009-10-20 15:25:57 PDT
(In reply to comment #3)
> Hmm. Adam reviewed at the same time....
> 
> Here were my comments:
> 
> Two minor nits to fix on landing.
> 
> > Index: WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
> >  #include "ScriptSourceCode.h"
> >  #include "SerializedScriptValue.h"
> >  #include "Settings.h"
> > +#include "Storage.h"
> >  #include "SharedWorkerRepository.h"
> 
> "Storage" should be after "SharedWorker..."

Oops!

> > Index: WebCore/storage/Storage.cpp
> > +#if USE(V8)
> > +// TODO: Remove once these features are turned on by default in Chromium.
> 
> s/TODO/FIXME/

Yeah...I actually noticed this myself and was going to fix on landing.
Comment 5 Jeremy Orlow 2009-11-05 11:29:31 PST
Committed r50569: <http://trac.webkit.org/changeset/50569>
Comment 6 Jeremy Orlow 2009-11-09 11:12:29 PST
This was backed out because it broke QT.  Now that they no-longer depend on it (https://bugs.webkit.org/show_bug.cgi?id=31249) it should go back in.
Comment 7 Jeremy Orlow 2009-11-09 11:13:30 PST
Comment on attachment 41525 [details]
Patch v1

To be clear, the part that was backed out was just removing the session storage settings flag.  I'll roll a new patch.
Comment 8 Jeremy Orlow 2009-11-09 16:49:19 PST
On second thought, it'd be better to split this bug out: https://bugs.webkit.org/show_bug.cgi?id=31279