[Webkit-unassigned] [Bug 44891] [V8] Share bindings logic for location.replace between V8 and JSC

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66172|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #10 from Adam Barth <abarth at webkit.org>  2010-09-01 00:24:25 PST ---
(From update of attachment 66172)
View in context: https://bugs.webkit.org/attachment.cgi?id=66172&action=prettypatch

Looks great.  Give Sam a day or two to comment before landing the patch though.

> WebCore/bindings/generic/GenericBinding.h:35
> +#include "Frame.h"
> +#include "FrameLoader.h"
These don't seem needed anymore.

> WebCore/bindings/generic/GenericBinding.h:46
> -class State {};
> +class State {
> +};
This change is correct, but probably not needed.

> WebCore/bindings/js/JSLocationCustom.cpp:192
> +    JSBindingState state(exec);
> +    JSBinding::Frame::navigateIfAllowed(&state, frame, url, lockHistory ? JSBinding::Frame::LockedHistory : JSBinding::Frame::UnlockedHistory, lockBackForwardList ? JSBinding::Frame::LockedBackForwardList : JSBinding::Frame::UnlockedBackForwardList);
What do you think?  Is this better?

> WebCore/bindings/js/specialization/JSBindingState.h:51
> +    explicit State(JSC::ExecState* exec)
> +        : m_exec(exec) {}
Usually we'll put these braces on their own lines.

> WebCore/bindings/v8/V8Utilities.cpp:119
> +    return V8Binding::Frame::navigateIfAllowed(V8BindingState::Only(), frame, url, lockHistory ? V8Binding::Frame::LockedHistory : V8Binding::Frame::UnlockedHistory, lockBackForwardList ? V8Binding::Frame::LockedBackForwardList : V8Binding::Frame::UnlockedBackForwardList);
I think to make this work, we'd have to put these enums in WebCore, probably in FrameLoader and push them through the loading system.  It's probably worth doing, but maybe not in this patch.

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