Bug 38174 – Add CallWith=DynamicFrame to CodeGenerator

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Add CallWith=DynamicFrame to CodeGenerator

[Adam Barth]

Created attachment 54385 [details]
Patch

[Adam Barth]

Comment on attachment 54385 [details]
Patch

Go, go EWS

[Eric Seidel (no email)]

Comment on attachment 54385 [details]
Patch

You need to check the return of toDynamicFrame().

[Adam Barth]

Created attachment 54386 [details]
Patch

[WebKit Review Bot]

Attachment 54386 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:140:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:142:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:149:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:151:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:158:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:160:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:167:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:169:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:176:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:178:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:71:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:74:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:77:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:80:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h:83:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/JS/JSTestObj.cpp:140:  Missing spaces around |  [whitespace/operators] [3]
WebCore/bindings/scripts/test/JS/JSTestObj.cpp:141:  Missing spaces around |  [whitespace/operators] [3]
WebCore/bindings/scripts/test/JS/JSTestObj.cpp:142:  Missing spaces around |  [whitespace/operators] [3]
WebCore/bindings/scripts/test/JS/JSTestObj.cpp:143:  Missing spaces around |  [whitespace/operators] [3]
WebCore/bindings/scripts/test/JS/JSTestObj.cpp:144:  Missing spaces around |  [whitespace/operators] [3]
Total errors found: 20 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Seidel (no email)]

Comment on attachment 54386 [details]
Patch

Looks sane.  I look forward to a patch with wider deployment.

[Adam Barth]

Comment on attachment 54386 [details]
Patch

Moving back to r? to get input from chromium-ews

[Adam Barth]

Created attachment 54387 [details]
Patch

[WebKit Review Bot]

Attachment 54386 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1786142

[Adam Barth]

Committed r58295: <http://trac.webkit.org/changeset/58295>

[WebKit Review Bot]

http://trac.webkit.org/changeset/58295 might have broken GTK Linux 32-bit Release

[Eric Seidel (no email)]

Looks like the bot is just upset at itself.  I don't think this change is actually causing the failure.

[Eric Seidel (no email)]

The failures have persisted.  Either this was a real Gtk-only regession, or the Gtk bot is still wedged somehow.

[Adam Barth]

Maybe a dependency issue?

[Eric Seidel (no email)]

Kov says this is a known problem and his bot is just mad at the world.  He's fixing.

[Yaar Schnitman]

Awesomeness!

Can you elaborate where CallWith=DynamicFrame is going to be used? Is this for places we are currently passing ScriptExecutionContext, or also for security checks?

[Adam Barth]

> Can you elaborate where CallWith=DynamicFrame is going to be used?

It's for places that need to pass a Frame* to WebCore and were they need the frame to come from the dynamic scope (instead of the lexical scope).  LexicalFrame should be more common, but the first example I happened to see was the DynamicFrame.

> Is this for
> places we are currently passing ScriptExecutionContext, or also for security
> checks?

No.  ScriptExecutionContext is a Document-lifetime object (actually, it often is the document).  Frame is a Frame-lifetime object.  These objects hare highly not interchangeable.  :)