Bug 40558 - [DRT/Chromium] Upstream TestShellDevTools for Chromium DRT
Summary: [DRT/Chromium] Upstream TestShellDevTools for Chromium DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Roland Steiner
URL:
Keywords: GoogleBug
Depends on:
Blocks: 28395
  Show dependency treegraph
 
Reported: 2010-06-14 00:57 PDT by Roland Steiner
Modified: 2010-07-06 05:38 PDT (History)
7 users (show)

See Also:


Attachments
patch - Upstreamed DevTools for Chromium DRT (36.06 KB, patch)
2010-06-18 02:05 PDT, Roland Steiner
dglazkov: review-
Details | Formatted Diff | Diff
patch - snapshot of current state (37.06 KB, patch)
2010-07-02 05:51 PDT, Roland Steiner
rolandsteiner: commit-queue-
Details | Formatted Diff | Diff
patch - snapshot of current state (removed debugging code, fixed modes) (36.21 KB, patch)
2010-07-02 06:36 PDT, Roland Steiner
tkent: review-
Details | Formatted Diff | Diff
patch - snapshot of current state (fixed style bugs and addressed review remarks) (35.49 KB, patch)
2010-07-04 23:18 PDT, Roland Steiner
no flags Details | Formatted Diff | Diff
patch - snapshot of current state (removed unnecessary struct) (35.20 KB, patch)
2010-07-04 23:56 PDT, Roland Steiner
tkent: review-
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch - snapshot of current state (added tkent's fix, NOBODY-fied the reviewer in the ChangeLogs) (35.64 KB, patch)
2010-07-06 01:18 PDT, Roland Steiner
tkent: review-
Details | Formatted Diff | Diff
patch - snapshot of current state (cleaned up, removed webkit.gyp modification) (33.76 KB, patch)
2010-07-06 04:52 PDT, Roland Steiner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2010-06-14 00:57:55 PDT
Upstream TestShellDevTools for Chromium DRT
Comment 1 Roland Steiner 2010-06-18 02:05:24 PDT
Created attachment 59084 [details]
patch - Upstreamed DevTools for Chromium DRT

First version, tested under Windows (running LayoutTests/inspector).

I'm unsure about the modification to WebKit.gyp (adding include dir for header generated from resources), i.e., whether to really put that there, and for which platforms.

Difference to test shell version: on showDevTools() the test shell version instantiated a separate TestShell instance using weak pointers.
This doesn't seem to be necessary for the DRT implementation (using a single TestShell instance and a simple WebViewHost* instead).
Comment 2 WebKit Review Bot 2010-06-18 02:09:26 PDT
Attachment 59084 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/DumpRenderTree/chromium/TestShell.cpp:65:  webkit_glue is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Review Bot 2010-06-18 03:08:46 PDT
Attachment 59084 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/3308316
Comment 4 Kent Tamura 2010-06-18 03:58:00 PDT
Comment on attachment 59084 [details]
patch - Upstreamed DevTools for Chromium DRT

http://wkrietveld.appspot.com/40558/diff/1/4
File WebKitTools/ChangeLog (right):

http://wkrietveld.appspot.com/40558/diff/1/4#newcode8
WebKitTools/ChangeLog:8: Upstream DevTools for Chromium DRT.
Please write the base Chromium revision of new files.

http://wkrietveld.appspot.com/40558/diff/1/4#newcode26
WebKitTools/ChangeLog:26: * DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp: Added.
The name "WebDevToolsDRTAgent" looks curious to me.  I feel "WebDevToolsAgentDRT" or "DRT(Web)DevToolsAgent" is better.
Same for other new files.

http://wkrietveld.appspot.com/40558/diff/1/6
File WebKitTools/DumpRenderTree/chromium/EventSender.cpp (right):

http://wkrietveld.appspot.com/40558/diff/1/6#newcode63
WebKitTools/DumpRenderTree/chromium/EventSender.cpp:63: #pragma warning(disable : 4355)
It's better to have this in config.h.

http://wkrietveld.appspot.com/40558/diff/1/8
File WebKitTools/DumpRenderTree/chromium/TestShell.cpp (right):

http://wkrietveld.appspot.com/40558/diff/1/8#newcode57
WebKitTools/DumpRenderTree/chromium/TestShell.cpp:57: #include "webkit/glue/webkit_glue.h"
Do not use webkti_glue.h
If you can't remove it easily, add a FIME comment.

http://wkrietveld.appspot.com/40558/diff/1/8#newcode135
WebKitTools/DumpRenderTree/chromium/TestShell.cpp:135: FilePath dirExe;
Do not use FilePath.

http://wkrietveld.appspot.com/40558/diff/1/9
File WebKitTools/DumpRenderTree/chromium/TestShell.h (right):

http://wkrietveld.appspot.com/40558/diff/1/9#newcode146
WebKitTools/DumpRenderTree/chromium/TestShell.h:146: void initializeWebDevToolsDRTAgent(WebKit::WebView* webView);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/9#newcode147
WebKitTools/DumpRenderTree/chromium/TestShell.h:147: void createWebDevToolsDRTClient(WebDevToolsDRTAgent *agent);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/10
File WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp (right):

http://wkrietveld.appspot.com/40558/diff/1/10#newcode37
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:37: #include "base/message_loop.h"
Do not use message_loop.h.

http://wkrietveld.appspot.com/40558/diff/1/10#newcode38
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:38: #include "base/string_piece.h"
Do not use string_pieace.h.

http://wkrietveld.appspot.com/40558/diff/1/10#newcode45
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:45: #include "webkit/glue/webkit_glue.h"
Do not use webkit_glue.h.

http://wkrietveld.appspot.com/40558/diff/1/10#newcode52
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:52: using WebKit::WebCString;
"using namespace WebKit" is enough.

http://wkrietveld.appspot.com/40558/diff/1/10#newcode85
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:85: // FIXME: implement this. - original TODO(loislo)
Remove TODO(name) comment

http://wkrietveld.appspot.com/40558/diff/1/10#newcode90
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:90: base::StringPiece injectJSWebkit = webkit_glue::GetDataResource(IDR_DEVTOOLS_INJECT_WEBKIT_JS);
We should not use StringPieace and webkit_glue in WebKit code.
So how about adding following function to webkit_support?
  WebCString webkit_support::GetDevToolsInjectedScriptSource();
  WebCString webkit_support::GetDevToolsInjectedScriptDispatcherSource();
  WebCString webkit_support::GetDevToolsDebuggerScriptSource();

http://wkrietveld.appspot.com/40558/diff/1/10#newcode108
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:108: MessageLoop::current()->PostDelayedTask(
Do not use MessageLoop.  Use webkit_support::PostDelayedTaskFromhere() instead.

http://wkrietveld.appspot.com/40558/diff/1/10#newcode167
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.cpp:167: void WebDevToolsDRTAgent::dispatchMessageLoop()
Use webkit_support::DispatchMessageLoop() instead.

http://wkrietveld.appspot.com/40558/diff/1/11
File WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h (right):

http://wkrietveld.appspot.com/40558/diff/1/11#newcode34
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:34: #include "base/logging.h"
Do not use base/logging.h.

http://wkrietveld.appspot.com/40558/diff/1/11#newcode57
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:57: WebDevToolsDRTAgent(WebKit::WebView* web_view);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/11#newcode61
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:61: virtual void sendMessageToFrontend(const WebKit::WebDevToolsMessageData& data);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/11#newcode69
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:69: void asyncCall(const WebDevToolsDRTCallArgs& args);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/11#newcode71
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:71: void attach(WebDevToolsDRTClient* client);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/11#newcode72
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:72: void detach(WebDevToolsDRTClient* client);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/11#newcode78
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTAgent.h:78: void call(const WebDevToolsDRTCallArgs& args);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/12
File WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTCallArgs.cpp (right):

http://wkrietveld.appspot.com/40558/diff/1/12#newcode35
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTCallArgs.cpp:35: int WebDevToolsDRTCallArgs::m_callsCount = 0;
Can we merge this to another file?

http://wkrietveld.appspot.com/40558/diff/1/14
File WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.cpp (right):

http://wkrietveld.appspot.com/40558/diff/1/14#newcode37
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.cpp:37: #include "base/message_loop.h"
Do not use message_loop.h.

http://wkrietveld.appspot.com/40558/diff/1/14#newcode113
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.cpp:113: MessageLoop::current()->PostDelayedTask(
Use webkit_support::PostDelayedTaskFromHere().

http://wkrietveld.appspot.com/40558/diff/1/15
File WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.h (right):

http://wkrietveld.appspot.com/40558/diff/1/15#newcode36
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.h:36: #undef LOG
is this needed?

http://wkrietveld.appspot.com/40558/diff/1/15#newcode57
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.h:57: WebDevToolsDRTClient(WebDevToolsDRTAgent* agent, WebKit::WebView* web_view);
Argument names are not needed.

http://wkrietveld.appspot.com/40558/diff/1/15#newcode61
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.h:61: virtual void sendMessageToAgent(const WebKit::WebDevToolsMessageData& data);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/15#newcode69
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.h:69: void asyncCall(const WebDevToolsDRTCallArgs& args);
Argument name is not needed.

http://wkrietveld.appspot.com/40558/diff/1/15#newcode74
WebKitTools/DumpRenderTree/chromium/WebDevToolsDRTClient.h:74: void call(const WebDevToolsDRTCallArgs& args);
Argument name is not needed.
Comment 5 Dimitri Glazkov (Google) 2010-06-18 08:01:25 PDT
Comment on attachment 59084 [details]
patch - Upstreamed DevTools for Chromium DRT

r- based on comments from tkent.
Comment 6 Roland Steiner 2010-07-02 05:51:01 PDT
Created attachment 60360 [details]
patch - snapshot of current state

Temporary snap-shot of current state. It's not meant as a final patch since there are some remaining issues that I'm currently stuck on:

For some reason, the event handlers are not properly called, esp. WebDevToolsFrontendImpl::frontendLoaded() is never called from dispatchWindowLoadEvent() in Document::implicitClose() (via loadedCallback in the auto-generated V8InspectorFrontendHost.cpp).

Hence no request for review (the patch also contains 2 debugging MessageBox calls, as well as those annoying 755 file modes).
Comment 7 Kent Tamura 2010-07-02 06:01:38 PDT
(In reply to comment #6)
> Created an attachment (id=60360) [details]
> patch - snapshot of current state

> Hence no request for review (the patch also contains 2 debugging MessageBox calls, as well as those annoying 755 file modes).

I think we had better commit the code as soon as possible even if the code is incomplete so that other Chromium developers can investigate problems.
Comment 8 Dimitri Glazkov (Google) 2010-07-02 06:36:13 PDT
I wonder if it is time to involve the inspector folks who wrotenthis code in the first place.
Comment 9 Roland Steiner 2010-07-02 06:36:30 PDT
Created attachment 60363 [details]
patch - snapshot of current state (removed debugging code, fixed modes)

Based on Kent-san's comment, uploaded a cleaned-up version of the current state, without debugging message boxes, and with the filemodes corrected.

Apart from the code issues, as indicated with my first patch, please also check whether my modifications to webkit.gyp are OK. I feel that having the include dirs modification just for Windows is probably not correct.
Comment 10 WebKit Review Bot 2010-07-02 06:38:06 PDT
Attachment 60363 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebKitTools/DumpRenderTree/chromium/TestShell.cpp:35:  Alphabetical sorting problem.  [build/include_order] [4]
WebKitTools/DumpRenderTree/chromium/TestShell.cpp:57:  Should have a space between // and comment  [whitespace/comments] [4]
WebKitTools/DumpRenderTree/chromium/DRTDevToolsClient.h:36:  Should have a space between // and comment  [whitespace/comments] [4]
WebKitTools/DumpRenderTree/chromium/LayoutTestController.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
WebKitTools/DumpRenderTree/chromium/DRTDevToolsAgent.h:34:  Should have a space between // and comment  [whitespace/comments] [4]
WebKitTools/DumpRenderTree/chromium/DRTDevToolsAgent.h:37:  Should have a space between // and comment  [whitespace/comments] [4]
WebKitTools/DumpRenderTree/chromium/DRTDevToolsAgent.cpp:58:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 7 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Pavel Feldman 2010-07-02 06:45:46 PDT
What is the status of this code? Should it be reviewed? Is it a pure port or does it do anything?
Comment 12 Roland Steiner 2010-07-02 06:53:02 PDT
@Pavel: It's the upstreamed version of test_shell_devtools. 

The code basically seems to get called and instantiated fine, AFAICT - it just doesn't currently do anything, because it hangs due to the problems I described.
Comment 13 Kent Tamura 2010-07-04 21:09:43 PDT
Comment on attachment 60363 [details]
patch - snapshot of current state (removed debugging code, fixed modes)

Anyway, please fix style errors.


WebKitTools/DumpRenderTree/chromium/DRTDevToolsClient.cpp:45
 +  using WebKit::WebDevToolsAgent;
We prefer "using namespace WebKit;" in .cpp files.



WebKitTools/DumpRenderTree/chromium/DRTDevToolsClient.cpp:112
 +  void DRTDevToolsClient::call(const DRTDevToolsCallArgs &args)
"DRTDevToolsCallArgs &arg" -> "DRTDevToolsCallArgs& arg"


WebKitTools/DumpRenderTree/chromium/TestShell.cpp:124
 +  void TestShell::createDRTDevToolsClient(DRTDevToolsAgent *agent)
"DRTDevToolsAgent *agent" -> "DRTDevToolsAgent* agent"


WebKitTools/DumpRenderTree/chromium/config.h:44
 +  #define NOTREACHED() DCHECK(false)
Do we still need this?


WebKitTools/DumpRenderTree/chromium/EventSender.cpp:61
 +  #endif // OS(WINDOWS)
This patch doesn't need this change.
Comment 14 Roland Steiner 2010-07-04 23:18:00 PDT
Created attachment 60495 [details]
patch - snapshot of current state (fixed style bugs and addressed review remarks)

New patch: addressed style issues and last review remarks. Code itself is unchanged.
Comment 15 Kent Tamura 2010-07-04 23:25:34 PDT
Comment on attachment 60495 [details]
patch - snapshot of current state (fixed style bugs and addressed review remarks)

WebKitTools/DumpRenderTree/chromium/TestShell.h:88
 +      TestShell* m_testShell;
We don't prepend "m_" to public data members.
Comment 16 Roland Steiner 2010-07-04 23:56:55 PDT
Created attachment 60497 [details]
patch - snapshot of current state (removed unnecessary struct)

Patch: removed unnecessary struct 'TestShellWeakReference'

(also, resubmitted to use commit-queue)
Comment 17 WebKit Commit Bot 2010-07-05 01:56:57 PDT
Comment on attachment 60497 [details]
patch - snapshot of current state (removed unnecessary struct)

Rejecting patch 60497 from commit-queue.

Unexpected failure when processing patch!  Please file a bug against webkit-patch.
Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 60497, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=40558&ctype=xml
Processing 1 patch from 1 bug.
Cleaning working directory
Processing patch 60497 from bug 40558.
Tamura Kent found in /Users/eseidel/Projects/CommitQueue/WebKit/chromium/ChangeLog does not appear to be a valid reviewer according to committers.py.
ERROR: /Users/eseidel/Projects/CommitQueue/WebKit/chromium/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Comment 18 Kent Tamura 2010-07-05 03:35:35 PDT
Comment on attachment 60497 [details]
patch - snapshot of current state (removed unnecessary struct)

I don't know why CQ rejects the patch :-<

WebKitTools/DumpRenderTree/chromium/TestShell.cpp:125
 +      m_drtDevToolsClient.set(new DRTDevToolsClient(agent, webView()));
webView() should be m_devTools->webView().
This is the root cause of no frontendLoaded() called.
Comment 19 Kent Tamura 2010-07-05 04:19:19 PDT
Comment on attachment 60497 [details]
patch - snapshot of current state (removed unnecessary struct)

WebKit/chromium/ChangeLog:3
 +          Reviewed by Tamura Kent.
You shouldn't put a reviewer name to a uploading patch.
Comment 20 Roland Steiner 2010-07-06 01:18:20 PDT
Created attachment 60602 [details]
patch - snapshot of current state (added tkent's fix, NOBODY-fied the reviewer in the ChangeLogs)

Made the suggested change to webView, but test still time out (further debugging pending). Also reset reviewer back to "NOBODY (OOPS!)" to mollify the CQ.
Comment 21 Kent Tamura 2010-07-06 01:26:44 PDT
Comment on attachment 60602 [details]
patch - snapshot of current state (added tkent's fix, NOBODY-fied the reviewer in the ChangeLogs)



> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> index 62ad190..fa7d242 100644
> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,72 @@
> +2010-07-05  Roland Steiner  <rolandsteiner@chromium.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Bug 40558 - [DRT/Chromium] Upstream TestShellDevTools for Chromium DRT
> +        https://bugs.webkit.org/show_bug.cgi?id=40558
> +
> +        Upstream DevTools for Chromium DRT.
> +        (original Chromium files rev. 51287)
> +
> +        * DumpRenderTree/DumpRenderTree.gypi:
> +        * DumpRenderTree/chromium/DRTDevToolsAgent.cpp: Added.
> +        (DRTDevToolsAgent::DRTDevToolsAgent):
> +        (DRTDevToolsAgent::setWebView):
> +        (DRTDevToolsAgent::sendMessageToFrontend):
> +        (DRTDevToolsAgent::forceRepaint):
> +        (DRTDevToolsAgent::runtimeFeatureStateChanged):
> +        (DRTDevToolsAgent::injectedScriptSource):
> +        (DRTDevToolsAgent::injectedScriptDispatcherSource):
> +        (DRTDevToolsAgent::debuggerScriptSource):
> +        (DRTDevToolsAgent::asyncCall):
> +        (DRTDevToolsAgent::call):
> +        (DRTDevToolsAgent::webDevToolsAgent):
> +        (DRTDevToolsAgent::attach):
> +        (DRTDevToolsAgent::detach):
> +        (DRTDevToolsAgent::setTimelineProfilingEnabled):
> +        (DRTDevToolsAgent::evaluateInWebInspector):
> +        (DRTDevToolsAgent::dispatchMessageLoop):
> +        * DumpRenderTree/chromium/DRTDevToolsAgent.h: Added.
> +        (DRTDevToolsAgent::~DRTDevToolsAgent):
> +        (DRTDevToolsAgent::hostIdentifier):
> +        * DumpRenderTree/chromium/DRTDevToolsCallArgs.cpp:
> +        * DumpRenderTree/chromium/DRTDevToolsCallArgs.h:
> +        (DRTDevToolsCallArgs::DRTDevToolsCallArgs):
> +        (DRTDevToolsCallArgs::~DRTDevToolsCallArgs):
> +        (DRTDevToolsCallArgs::callsCount):
> +        * DumpRenderTree/chromium/DRTDevToolsClient.cpp: Added.
> +        (DRTDevToolsClient::DRTDevToolsClient):
> +        (DRTDevToolsClient::~DRTDevToolsClient):
> +        (DRTDevToolsClient::sendMessageToAgent):
> +        (DRTDevToolsClient::sendDebuggerCommandToAgent):
> +        (DRTDevToolsClient::activateWindow):
> +        (DRTDevToolsClient::closeWindow):
> +        (DRTDevToolsClient::dockWindow):
> +        (DRTDevToolsClient::undockWindow):
> +        (DRTDevToolsClient::asyncCall):
> +        (DRTDevToolsClient::call):
> +        (DRTDevToolsClient::allMessagesProcessed):
> +        * DumpRenderTree/chromium/DRTDevToolsClient.h:
> +        * DumpRenderTree/chromium/EventSender.cpp:
> +        * DumpRenderTree/chromium/LayoutTestController.cpp:
> +        (LayoutTestController::LayoutTestController):
> +        (LayoutTestController::closeWebInspector):
> +        (LayoutTestController::setTimelineProfilingEnabled):
> +        (LayoutTestController::evaluateInWebInspector):
> +        * DumpRenderTree/chromium/LayoutTestController.h:
> +        * DumpRenderTree/chromium/TestShell.cpp:
> +        (TestShell::TestShell):
> +        (TestShell::~TestShell):
> +        (TestShell::createDRTDevToolsClient):
> +        (TestShell::showDevTools):
> +        (TestShell::closeDevTools):
> +        (TestShell::runFileTest):
> +        (TestShell::createNewWindow):
> +        * DumpRenderTree/chromium/TestShell.h:
> +        (TestShell::drtDevToolsAgent):
> +        (TestShell::drtDevToolsClient):
> +        * DumpRenderTree/chromium/config.h:
> +
>  2010-07-05  Csaba Osztrogonác  <ossy@webkit.org>
>  
>          Reviewed by Eric Seidel.
> @@ -40,7 +109,6 @@
>  
>          * Scripts/webkitpy/common/checkout/scm.py:
>          * Scripts/webkitpy/common/checkout/scm_unittest.py:
> -
>  2010-07-03  Patrick Gansterer  <paroga@paroga.com>

Do not change unrelated part of ChangeLog.


WebKit/chromium/WebKit.gyp:633
 +                          '<(SHARED_INTERMEDIATE_DIR)/webkit', # for a header generated by grit
Is this needed?
Comment 22 Eric Seidel (no email) 2010-07-06 03:16:22 PDT
Comment on attachment 60495 [details]
patch - snapshot of current state (fixed style bugs and addressed review remarks)

Cleared Kent Tamura's review+ from obsolete attachment 60495 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 23 Roland Steiner 2010-07-06 04:52:56 PDT
Created attachment 60619 [details]
patch - snapshot of current state (cleaned up, removed webkit.gyp modification)

You're right, the WebKit.gyp modification is really no longer necessary.

new, cleaned up patch.
Comment 24 WebKit Commit Bot 2010-07-06 05:11:18 PDT
Comment on attachment 60619 [details]
patch - snapshot of current state (cleaned up, removed webkit.gyp modification)

Clearing flags on attachment: 60619

Committed r62545: <http://trac.webkit.org/changeset/62545>
Comment 25 WebKit Commit Bot 2010-07-06 05:11:24 PDT
All reviewed patches have been landed.  Closing bug.
Comment 26 WebKit Review Bot 2010-07-06 05:38:15 PDT
http://trac.webkit.org/changeset/62545 might have broken GTK Linux 64-bit Debug