Bug 37561

Summary: [chromium] Add TestShellGtk.cpp so we can link on Linux
Product: WebKit Reporter: Tony Chang <tony>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dglazkov, fishd, tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 35902    
Attachments:
Description Flags
Patch
none
Patch dglazkov: review+, commit-queue: commit-queue-

Description Tony Chang 2010-04-14 00:00:59 PDT
[chromium] Add TestShellGtk.cpp so we can link on Linux
Comment 1 Tony Chang 2010-04-14 00:01:32 PDT
Created attachment 53322 [details]
Patch
Comment 2 Tony Chang 2010-04-14 00:03:39 PDT
Kent-san would make a good reviewer for this.
Comment 3 Kent Tamura 2010-04-14 11:23:44 PDT
Comment on attachment 53322 [details]
Patch

> --- a/WebKitTools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
> +++ b/WebKitTools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
> @@ -91,6 +91,7 @@
>                  '../chromium/TestNavigationController.h',
>                  '../chromium/TestShell.cpp',
>                  '../chromium/TestShell.h',
> +                '../chromium/TestShellGtk.cpp',
>                  '../chromium/TestShellMac.mm',
>                  '../chromium/TextInputController.cpp',
>                  '../chromium/TextInputController.h',

Don't we need to have 'condition' block to exclude TestShellGtk.cpp for Mac ad TestShellMac.mm for non-Mac UNIX?

C++ code looks ok.
Comment 4 Tony Chang 2010-04-14 18:21:24 PDT
Created attachment 53395 [details]
Patch
Comment 5 Tony Chang 2010-04-14 18:23:16 PDT
Comment on attachment 53395 [details]
Patch

(In reply to comment #3)
> (From update of attachment 53322 [details])
> > --- a/WebKitTools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
> > +++ b/WebKitTools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
> > @@ -91,6 +91,7 @@
> >                  '../chromium/TestNavigationController.h',
> >                  '../chromium/TestShell.cpp',
> >                  '../chromium/TestShell.h',
> > +                '../chromium/TestShellGtk.cpp',
> >                  '../chromium/TestShellMac.mm',
> >                  '../chromium/TextInputController.cpp',
> >                  '../chromium/TextInputController.h',
> 
> Don't we need to have 'condition' block to exclude TestShellGtk.cpp for Mac ad
> TestShellMac.mm for non-Mac UNIX?

Yes, you're right.  I added exclude blocks for files ending in Gtk, Linux, Mac, and Win.  We didn't need it before for Mac.mm because .mm is excluded in common.gypi.
Comment 6 Kent Tamura 2010-04-14 18:26:53 PDT
Comment on attachment 53395 [details]
Patch

LGTM.
Comment 7 Dimitri Glazkov (Google) 2010-04-15 09:23:07 PDT
Comment on attachment 53395 [details]
Patch

yay!
Comment 8 WebKit Commit Bot 2010-04-15 09:54:57 PDT
Comment on attachment 53395 [details]
Patch

Rejecting patch 53395 from commit-queue.

Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Dimitri Glazkov', '--force']" exit_code: 1
patching file WebKitTools/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKitTools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp
Hunk #2 FAILED at 111.
1 out of 2 hunks FAILED -- saving rejects to file WebKitTools/DumpRenderTree/DumpRenderTree.gyp/DumpRenderTree.gyp.rej
patching file WebKitTools/DumpRenderTree/chromium/TestShellGtk.cpp

Full output: http://webkit-commit-queue.appspot.com/results/1586472
Comment 9 Tony Chang 2010-04-15 18:16:51 PDT
Committed r57703: <http://trac.webkit.org/changeset/57703>