Currently ScriptCallStack is created based on current Frame's main world context. This is wrong since current context maybe one from isolated world.
Created attachment 45594 [details] patch
Created attachment 45595 [details] patch
style-queue ran check-webkit-style on attachment 45595 [details] without any errors.
Comment on attachment 45595 [details] patch I like the premise of the patch. I don't like that we're adding extra methods to already humongous V8Proxy and V8IsolatedWorld. ScriptState is supposed to be a thin wrapper, and I certainly don't want us to carry it around in either of these instances. Can you rework it perhaps to just add static create methods to ScriptState, and put world-aware logic there?
(In reply to comment #4) > (From update of attachment 45595 [details]) > I like the premise of the patch. I don't like that we're adding extra methods > to already humongous V8Proxy and V8IsolatedWorld. ScriptState is supposed to be > a thin wrapper, and I certainly don't want us to carry it around in either of > these instances. > > Can you rework it perhaps to just add static create methods to ScriptState, and > put world-aware logic there? ScriptState unlike other Script* objetcs is not a value type. I don't want to manage ScriptState life time and my point was to cache pointer to ScriptState on the objects that can be casted to ScriptState* This way the ScriptState instance would live as long as the original object stays alive. Otherwise if one needed to store a pointer to ScriptState from the call stack he would have to make a copy of the ScriptCallStack.state() since the original ScriptState object would be destroyed along with stack allocated ScriptCallStack object. I guess original ScriptState design assumes that clients should be able to keep pointer to ScriptState instance while original context exists. Please correct me if I'm wrong.
Created attachment 45617 [details] patch
Does this change have an observable change in behavior? If so, we'll need a test.
Attachment 45617 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bindings/v8/ScriptCallStack.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1
(In reply to comment #7) > Does this change have an observable change in behavior? If so, we'll need a > test. With current implementation of InspectorController the change should not affect observable behavior.
Created attachment 45618 [details] patch
style-queue ran check-webkit-style on attachment 45618 [details] without any errors.
Comment on attachment 45618 [details] patch r=me. Please file a bug regarding the new role of ScriptState.
(In reply to comment #12) > Please file a bug regarding the new role of ScriptState. Done: https://bugs.webkit.org/show_bug.cgi?id=33047
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/bindings/v8/ScriptCallStack.cpp M WebCore/bindings/v8/ScriptCallStack.h M WebCore/bindings/v8/ScriptController.cpp M WebCore/bindings/v8/ScriptController.h M WebCore/bindings/v8/V8IsolatedWorld.cpp M WebCore/bindings/v8/V8IsolatedWorld.h Committed r52653