On 2010/05/31 17:59:13, michaeln wrote:
> The use of the global g_cache_thread is kind of sketchy. The awkwardness of
> having test_shell vs the all new and improved dump render tree in the works at
> the same some is somewhat confusing... not to mention a separate target for
> test_shell_tests. We end up defining g_cache_thread in multiple .cc files for
> use depending on the target being built.
I'm not so happy about it either. I was trying to be very explicit about the
fact that now we have a global thread that should be managed as close to the
application main() as possible. The issue is that we have three test executables
that create an AppCache: test_shell, test_shell_tests (which we control) and RDT
(which we don't). I would prefer to have something like the code from patch set
#4, where the thread is encapsulated away, but that fails to connect DRT.
The real problem though (with the current approach, with our without
encapsulation) is that there is no connection between the thread and the request
context, so the request context can outlive the thread (destroyed with
TestWebkitClient), and even if there is no crash (the message loop is ref
counted), the destruction of the context will not work as expected, most likely
will hang.
I think it's better just to drop this and let these tests run with a memory-only
AppCache; we just have to makes sure that the differences in behavior are small
and covered by unit tests (the only thing should be that all responses are
synchronous but posted back again to make them look async, right?)

(I forgot to send this part)
http://codereview.chromium.org/2249005/diff/56001/57020
File webkit/tools/test_shell/test_shell_webkit_init.cc (right):
http://codereview.chromium.org/2249005/diff/56001/57020#newcode5
webkit/tools/test_shell/test_shell_webkit_init.cc:5: #include
"webkit/tools/test_shell/test_shell_webkit_init.h"
On 2010/05/31 17:59:13, michaeln wrote:
> Good call to split this into a .cc file seeing how big some of the method
bodies
> are. Would be really nice to create the new .cc file by svn copy'ing the .h
file
> so we retain the history of these method bodies.
yeah, I forgot about it. Thanks for pointing that out!.