Bug 38174 - Add CallWith=DynamicFrame to CodeGenerator
Summary: Add CallWith=DynamicFrame to CodeGenerator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-26 23:19 PDT by Adam Barth
Modified: 2010-04-27 14:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (27.71 KB, patch)
2010-04-26 23:20 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (28.64 KB, patch)
2010-04-26 23:26 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (28.63 KB, patch)
2010-04-26 23:52 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-04-26 23:19:05 PDT
Add CallWith=DynamicFrame to CodeGenerator
Comment 1 Adam Barth 2010-04-26 23:20:31 PDT
Created attachment 54385 [details]
Patch
Comment 2 Adam Barth 2010-04-26 23:21:40 PDT
Comment on attachment 54385 [details]
Patch

Go, go EWS
Comment 3 Eric Seidel (no email) 2010-04-26 23:24:30 PDT
Comment on attachment 54385 [details]
Patch

You need to check the return of toDynamicFrame().
Comment 4 Adam Barth 2010-04-26 23:26:57 PDT
Created attachment 54386 [details]
Patch
Comment 5 WebKit Review Bot 2010-04-26 23:32:23 PDT
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.
Comment 6 Eric Seidel (no email) 2010-04-26 23:38:15 PDT
Comment on attachment 54386 [details]
Patch

Looks sane.  I look forward to a patch with wider deployment.
Comment 7 Adam Barth 2010-04-26 23:40:56 PDT
Comment on attachment 54386 [details]
Patch

Moving back to r? to get input from chromium-ews
Comment 8 Adam Barth 2010-04-26 23:52:06 PDT
Created attachment 54387 [details]
Patch
Comment 9 WebKit Review Bot 2010-04-27 00:02:00 PDT
Attachment 54386 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/1786142
Comment 10 Adam Barth 2010-04-27 00:15:55 PDT
Committed r58295: <http://trac.webkit.org/changeset/58295>
Comment 11 WebKit Review Bot 2010-04-27 01:55:40 PDT
http://trac.webkit.org/changeset/58295 might have broken GTK Linux 32-bit Release
Comment 12 Eric Seidel (no email) 2010-04-27 01:57:18 PDT
Looks like the bot is just upset at itself.  I don't think this change is actually causing the failure.
Comment 13 Eric Seidel (no email) 2010-04-27 09:53:08 PDT
The failures have persisted.  Either this was a real Gtk-only regession, or the Gtk bot is still wedged somehow.
Comment 14 Adam Barth 2010-04-27 10:03:45 PDT
Maybe a dependency issue?
Comment 15 Eric Seidel (no email) 2010-04-27 10:04:31 PDT
Kov says this is a known problem and his bot is just mad at the world.  He's fixing.
Comment 16 Yaar Schnitman 2010-04-27 14:34:50 PDT
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?
Comment 17 Adam Barth 2010-04-27 14:47:45 PDT
> 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.  :)