Issue 2118001: Not using std::wstring to store progress status text because mshtml is sensit...

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 2118001: Not using std::wstring to store progress status text because mshtml is sensit... (Closed)

Created:
10 years, 7 months ago by tommi (sloooow) - chröme

Modified:
9 years, 7 months ago

Reviewers:
amit, ananta

CC:
chromium-reviews

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

Not using std::wstring to store progress status text because mshtml is sensitive to NULL vs L"". TEST=see bug BUG=44103 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47232

Patch Set 1 #

Created: 10 years, 7 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+69 lines, -14 lines)			Patch
	M	chrome_frame/urlmon_bind_status_callback.h	View		2 chunks	+51 lines, -5 lines	0 comments	Download
	M	chrome_frame/urlmon_bind_status_callback.cc	View		3 chunks	+18 lines, -9 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

amit

LGTM the change but wouldn' it be simpler to use a scoped_ptr<char> to store the ...

10 years, 7 months ago (2010-05-13 21:30:59 UTC) #2

tommi (sloooow) - chröme

sure - I can change it if you want. my head is probablyt too mshtml ...

10 years, 7 months ago (2010-05-13 21:32:52 UTC) #3

ananta

Nice find :) LGTM with Amit's suggestion of using scoped_ptr -Ananta

10 years, 7 months ago (2010-05-13 22:00:23 UTC) #4

amit

10 years, 7 months ago (2010-05-14 18:00:29 UTC) #6

Just going through the patch again and a couple of nits:
1. Progress::Progress - use lstrcpyn,
2. Is it possible to use just std::vector<Progress> if not
then std::vector<scoped_ptr<Progress>> if not
then std::vector<scoped_refptr<Progress>> ?
Then it would simplify things as we wouldn't need to keep track the
destruction.

On Thu, May 13, 2010 at 7:27 PM, <tommi@chromium.org> wrote:

> done
>
>
> http://codereview.chromium.org/2118001/show
>

Expand Messages | Collapse Messages