Don't use UpdateRegistryFromResourceS as it's not in 2013

Don't use UpdateRegistryFromResourceS as it's not in 2013

ATL version that ships with 2013 doesn't have a suffix. 
UpdateRegistryFromResource used to select either an S or
a D suffix based on the value of _ATL_STATIC_REGISTRY,
however in ATL 12 (vs2013), UpdateRegistryFromResource
now just means UpdateRegistryFromResourceS, and
UpdateRegistryFromResourceD is separate.

We want to keep building with 2010 and 2013, so #if based
on _ATL_VER.

R=ananta@chromium.org, robertshield@chromium.org
BUG=309197
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234439

[ananta, 2013-11-12 02:14:42]

LGTM if registering the dll continues to work

[robertshield, 2013-11-12 02:50:27]

Drive by question: I thought that UpdateRegistryFromResource was a macro that
resolved to UpdateRegistryFromResourceS or UpdateRegistryFromResourceD, both of
which are at least documented to be present in 2013:
http://msdn.microsoft.com/en-us/library/ctwwf1sb(v=vs.120).aspx (though I don't
have 2013 to check and the docs lie a lot).

I don't see _ATL_STATIC_REGISTRY defined anywhere, so won't this result in
UpdateRegistryFromResourceD being selected which will fail on a system that
doesn't have the atl dll on it?

[scottmg, 2013-11-12 04:04:03]

On 2013/11/12 02:50:27, robertshield wrote:
> Drive by question: I thought that UpdateRegistryFromResource was a macro that
> resolved to UpdateRegistryFromResourceS or UpdateRegistryFromResourceD, both
of
> which are at least documented to be present in 2013:
> http://msdn.microsoft.com/en-us/library/ctwwf1sb%28v=vs.120%29.aspx (though I
don't
> have 2013 to check and the docs lie a lot).
> 
> I don't see _ATL_STATIC_REGISTRY defined anywhere, so won't this result in
> UpdateRegistryFromResourceD being selected which will fail on a system that
> doesn't have the atl dll on it?

I'm a bit confused. Looking at what's in Microsoft Visual Studio
12.0\VC\atlmfc\include\atlbase.h:

"""
	// Resource-based Registration
	// Statically linking to Registry Ponent
	HRESULT WINAPI UpdateRegistryFromResource(
		_In_z_ LPCTSTR lpszRes,
		_In_ BOOL bRegister,
		_In_opt_ struct _ATL_REGMAP_ENTRY* pMapEntries = NULL) throw();
	HRESULT WINAPI UpdateRegistryFromResource(
		_In_ UINT nResID,
		_In_ BOOL bRegister,
		_In_opt_ struct _ATL_REGMAP_ENTRY* pMapEntries = NULL) throw();
"""

I'm not sure what 'Statically linking to Register Ponent' means.

[scottmg, 2013-11-12 04:06:28]

Oh, I see now. In 2010 it was based _ATL_STATIC_REGISTRY, but in 2013 they
"fixed" the unsuffixed version to mean S, and deleted the S version. Very
"helpful".

So, I shouldn't check this in. I'll have to add a define to chrome_tab.cc that's
conditional based on... something. Compiler version? ATL version?

[robertshield, 2013-11-12 04:15:55]

On Mon, Nov 11, 2013 at 11:06 PM, <scottmg@chromium.org> wrote:

> Oh, I see now. In 2010 it was based _ATL_STATIC_REGISTRY, but in 2013 they
> "fixed" the unsuffixed version to mean S, and deleted the S version. Very
> "helpful".
>
> So, I shouldn't check this in. I'll have to add a define to chrome_tab.cc
> that's
> conditional based on... something. Compiler version? ATL version?
>

ATL version sounds safer. _ATL_VER might be the thing?


>
> https://codereview.chromium.org/66583008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[scottmg, 2013-11-12 04:22:47]

On 2013/11/12 04:15:55, robertshield wrote:
> On Mon, Nov 11, 2013 at 11:06 PM, <mailto:scottmg@chromium.org> wrote:
> 
> > Oh, I see now. In 2010 it was based _ATL_STATIC_REGISTRY, but in 2013 they
> > "fixed" the unsuffixed version to mean S, and deleted the S version. Very
> > "helpful".
> >
> > So, I shouldn't check this in. I'll have to add a define to chrome_tab.cc
> > that's
> > conditional based on... something. Compiler version? ATL version?
> >
> 
> ATL version sounds safer. _ATL_VER might be the thing?
> 

Thanks, done.

[commit-bot: I haz the power, 2013-11-12 04:30:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottmg@chromium.org/66583008/90001

[commit-bot: I haz the power, 2013-11-12 06:47:12]

Change committed as 234439