Bug 6361 - Add plugin support to DumpRenderTree
Summary: Add plugin support to DumpRenderTree
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Geoffrey Garen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-01-03 17:19 PST by Geoffrey Garen
Modified: 2006-01-05 14:22 PST (History)
0 users

See Also:


Attachments
Fix (88.66 KB, patch)
2006-01-03 17:44 PST, Geoffrey Garen
eric: review-
Details | Formatted Diff | Diff
Fix 2 (43.90 KB, patch)
2006-01-05 09:52 PST, Geoffrey Garen
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2006-01-03 17:19:54 PST
We want this for layout testing plugin bugs.
Comment 1 Geoffrey Garen 2006-01-03 17:44:28 PST
Created attachment 5467 [details]
Fix

I pity da foo messin with netscape plugins.
Comment 2 Eric Seidel (no email) 2006-01-03 22:21:46 PST
Comment on attachment 5467 [details]
Fix

Index: WebKitTools/DumpRenderTree/DumpRenderTree.xcodeproj/ggaren.mode2

should be removed.

Wouldn't it be easier to add a hidden pref to WebKit to turn off the
don't-load-plugins-with-no-window optimization?

I'm not sure that the plugin should be under the APSL (I think that's what that
is?).Probably BSD like the rest of WebKit.

It looks like you used some tabs in at least the plugin code.

No {} around the one line if in plugin code.  Also, you're likely using C99 and
can put the int in the for declaration.

In c code our unofficall style is (I'm told) to put the * next to the variable
name, not the type.  c++ we put the * next to the type.  Confusing enough yet?

I think this one could use one more round of cleanup.  Removing the mode file
will reduce the size of the patch by 2/3rds :)	I've looked at it, but not
closely enough to really call this a review yet.
Comment 3 Geoffrey Garen 2006-01-05 09:52:23 PST
Created attachment 5498 [details]
Fix 2

>Index: WebKitTools/DumpRenderTree/DumpRenderTree.xcodeproj/ggaren.mode2
>should be removed.

Done. Sorry about that.

>Wouldn't it be easier to add a hidden pref to WebKit to turn off the
>don't-load-plugins-with-no-window optimization?

It's a neat idea, and I considered it, but ultimately I thought it would be
more complicated and unjustifiably risky. Risky because we would be changing
the behavior of WebKit/WebCore in an untested area of code without any visible
benefit to the user. (I can't think of any "real life" situation where we would
want to use that pref.) More complicated because the current solution is very
few lines of code, cleanly separated from the rest of the engine.

Another disadvantage to the hidden window I considered was performance of the
layout test engine. However, since the window is set to defer drawing, and it's
never made visible, the only overhead it incurs should be its allocation, which
isn't significant.

>I'm not sure that the plugin should be under the APSL (I think that's what
that
>is?).Probably BSD like the rest of WebKit.

I copied the licenses from the files in /Developer/Examples because this code
is an adaptation of that code. Is it OK for us to distribute non-BSD code?

>It looks like you used some tabs in at least the plugin code.

Oops. Should be fixed now.

>No {} around the one line if in plugin code.  Also, you're likely using C99
and
>can put the int in the for declaration.

Fixed.

>In c code our unofficall style is (I'm told) to put the * next to the variable

>name, not the type.  c++ we put the * next to the type.  Confusing enough yet?


Fixed.
Comment 4 Darin Adler 2006-01-05 10:16:23 PST
Comment on attachment 5498 [details]
Fix 2

Plenty we could tweak here, but it's test code and it looks good for that
purpose. r=me
Comment 5 Geoffrey Garen 2006-01-05 14:22:42 PST
Landed.