Enabled WebFrameTest, stubbed out a gtk test shell function.

Enabled WebFrameTest, refactored some test shell functions.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=4834

[Evan Stade, 2008-11-05 02:37:19]

1 more test passing

[brettw, 2008-11-05 14:53:41]

http://codereview.chromium.org/9391/diff/1/2
File webkit/glue/webframe_impl.cc (right):

http://codereview.chromium.org/9391/diff/1/2#newcode226
Line 226: const uint16_t* chars = reinterpret_cast<const
uint16_t*>(it.characters());
80 cols. You don't need the _t after the types which will fix the problem
(basictypes defines uint16 which is commonly used in Google).

[tony, 2008-11-05 17:12:27]

http://codereview.chromium.org/9391/diff/1/3
File webkit/tools/test_shell/test_shell_gtk.cc (right):

http://codereview.chromium.org/9391/diff/1/3#newcode48
Line 48: void TestShell::ShutdownTestShell() {
I think we just want to move this method from test_shell_win.cc into
test_shell.cc and #if defined(WIN_OS) the Ole stuff.

[Evan Stade, 2008-11-05 19:28:45]

added a little more refactoring

http://codereview.chromium.org/9391/diff/1/2
File webkit/glue/webframe_impl.cc (right):

http://codereview.chromium.org/9391/diff/1/2#newcode226
Line 226: const uint16_t* chars = reinterpret_cast<const
uint16_t*>(it.characters());
On 2008/11/05 14:53:42, brettw wrote:
> 80 cols. You don't need the _t after the types which will fix the problem
> (basictypes defines uint16 which is commonly used in Google).

Done.

http://codereview.chromium.org/9391/diff/1/3
File webkit/tools/test_shell/test_shell_gtk.cc (right):

http://codereview.chromium.org/9391/diff/1/3#newcode48
Line 48: void TestShell::ShutdownTestShell() {
On 2008/11/05 17:12:27, tony wrote:
> I think we just want to move this method from test_shell_win.cc into
> test_shell.cc and #if defined(WIN_OS) the Ole stuff.

Done.

http://codereview.chromium.org/9391/diff/7/11
File webkit/tools/test_shell/test_shell.cc (right):

http://codereview.chromium.org/9391/diff/7/11#newcode31
Line 31: #include "webkit/glue/webwidget.h"
#include "webkit/tools/test_shell/simple_resource_loader_bridge.h"

[tony, 2008-11-05 21:16:24]

LGTM, just one question below.

http://codereview.chromium.org/9391/diff/7/11
File webkit/tools/test_shell/test_shell.cc (right):

http://codereview.chromium.org/9391/diff/7/11#newcode175
Line 175: #endif
Why did this #endif move?

[Evan Stade, 2008-11-05 22:25:35]

updated base/logging.cc to avoid infinite loops

http://codereview.chromium.org/9391/diff/7/11
File webkit/tools/test_shell/test_shell.cc (right):

http://codereview.chromium.org/9391/diff/7/11#newcode175
Line 175: #endif
On 2008/11/05 21:16:24, tony wrote:
> Why did this #endif move?

There was a problem caused by logging, and this endif move fixed it, but it
wasn't the right solution. See changes to base/logging.cc

[tony, 2008-11-05 22:47:16]

LGTM

On 2008/11/05 22:25:35, Evan Stade wrote:
> updated base/logging.cc to avoid infinite loops
> 
> http://codereview.chromium.org/9391/diff/7/11
> File webkit/tools/test_shell/test_shell.cc (right):
> 
> http://codereview.chromium.org/9391/diff/7/11#newcode175
> Line 175: #endif
> On 2008/11/05 21:16:24, tony wrote:
> > Why did this #endif move?
> 
> There was a problem caused by logging, and this endif move fixed it, but it
> wasn't the right solution. See changes to base/logging.cc