Use LoadString instead of the ATL-dependent AtlGetStringResourceImage....

Use LoadString instead of the ATL-dependent AtlGetStringResourceImage.

BUG=5022
TEST=none

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

[James Hawkins, 2009-08-03 03:32:49]

Fixed a couple typos.

[darin (slow to review), 2009-08-03 05:57:10]

LGTM w/ one nit:

http://codereview.chromium.org/159779/diff/1006/9
File webkit/tools/test_shell/test_shell_win.cc (right):

http://codereview.chromium.org/159779/diff/1006/9#newcode689
Line 689: TCHAR localized[MAX_LOADSTRING];
nit: we generally avoid TCHAR in chromium code.  just go ahead with wchar_t, and
just stick to LoadString instead of LoadStringW.

[James Hawkins, 2009-08-03 06:15:11]

http://codereview.chromium.org/159779/diff/1006/9
File webkit/tools/test_shell/test_shell_win.cc (right):

http://codereview.chromium.org/159779/diff/1006/9#newcode689
Line 689: TCHAR localized[MAX_LOADSTRING];
On 2009/08/03 05:57:10, darin wrote:
> nit: we generally avoid TCHAR in chromium code.  just go ahead with wchar_t,
and
> just stick to LoadString instead of LoadStringW.

Should I change the three other instances of TCHAR in the file while I'm at it?

[darin (slow to review), 2009-08-03 06:35:56]

http://codereview.chromium.org/159779/diff/1006/9
File webkit/tools/test_shell/test_shell_win.cc (right):

http://codereview.chromium.org/159779/diff/1006/9#newcode689
Line 689: TCHAR localized[MAX_LOADSTRING];
On 2009/08/03 06:15:11, James Hawkins wrote:
> On 2009/08/03 05:57:10, darin wrote:
> > nit: we generally avoid TCHAR in chromium code.  just go ahead with wchar_t,
> and
> > just stick to LoadString instead of LoadStringW.
> 
> Should I change the three other instances of TCHAR in the file while I'm at
it?

That'd be great, thanks.  Feel free to punt if it gets messy.

[Peter Kasting, 2009-08-03 17:55:28]

LGTM with Darin's comment re: TCHAR addressed.  One question below.

http://codereview.chromium.org/159779/diff/1006/9
File webkit/tools/test_shell/test_shell_win.cc (right):

http://codereview.chromium.org/159779/diff/1006/9#newcode696
Line 696: return string16(localized, length);
Is this safe and sane when |length| == 0?  The old code wouldn't reach here
then, the new code does.

[darin (slow to review), 2009-08-03 18:13:08]

On Mon, Aug 3, 2009 at 10:55 AM, <pkasting@chromium.org> wrote:

> LGTM with Darin's comment re: TCHAR addressed.  One question below.
>
>
> http://codereview.chromium.org/159779/diff/1006/9
> File webkit/tools/test_shell/test_shell_win.cc (right):
>
> http://codereview.chromium.org/159779/diff/1006/9#newcode696
> Line 696: return string16(localized, length);
> Is this safe and sane when |length| == 0?  The old code wouldn't reach
> here then, the new code does.
>
>
so long as localized is non-null, it is well behaved.



>
> http://codereview.chromium.org/159779
>

[Peter Kasting, 2009-08-03 18:41:37]

LGTM

http://codereview.chromium.org/159779/diff/1010/13
File webkit/tools/test_shell/test_shell_win.cc (right):

http://codereview.chromium.org/159779/diff/1010/13#newcode523
Line 523: DWORD fileURLLength = sizeof(fileURL)/sizeof(fileURL[0]);
Nit: I'd just change this to INTERNET_MAX_URL_LENGTH, it seems more readable and
less error-prone.