On Mon, Sep 21, 2009 at 2:31 PM, Jeremy Orlow <jor...@chromium.org> wrote:

> I think we need to re-consider our practice of shipping beta/stable
> browsers with experimental features hidden behind flags--at least when they
> have any side-effects in JavaScript.  An example of where this has bitten us
> is http://code.google.com/p/chromium/issues/detail?id=22181
>
> Although part of the problem is the way they coded things (since both
> SessionStorage and LocalStorage use the Storage interface,
> its existence doesn't imply SessionStorage is necessarily available), this
> bug has pointed out a couple problems.  1) constructors are visible to
> javascript even when the feature is totally disabled.
>

If it's behind a flag, it shouldn't have been exposed, right?  On the
surface, it sounds like this code was only partially hidden behind the flag?

I think it would be a good idea to have a unit test which enumerates all
symbols that we're exposing into JS.  This should be a controlled list.

If we had this unit test, would it have caught this exposure?

Mike






>  2) When an object (like the Storage interface) wraps a NULL it shows up as
> null in JavaScript.  Since returning NULL/0 is the standard thing to do when
> the feature is disabled, this means that the functions return null when
> disabled at run time and undefined when disabled at compile time.  3) Even
> if we fixed the undefined problem, |'localStorage' in window| would still
> return true.
>
> We've been discussing these issues in a WebKit-dev thread (
> https://lists.webkit.org/pipermail/webkit-dev/2009-September/thread.html#9860)
> and although (2) is probably something we can solve without too much effort
> (Drew is going to look into it), (1) and (3) probably aren't worth changing
> if the runtime flag is just temporary.
>
> *As such, I feel fairly strongly that we should start disabling features
> behing a run-time flag with compile-time flags in future beta/stable builds
> if they have any side-effects in JavaScript.*  I'm working with Anthony
> LaForge to fix this for LocalStorage/SessionStorage right now.  I'm not sure
> if it's worth doing preemptively for other features, but it might be.  I
> definitely think we should do it the next time we cut a beta build.
>  Especially if there's a chance the beta will be an ancestor of a stable
> channel release.
>
> J
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
    http://groups.google.com/group/chromium-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to