Base: Fix FileVersionInfo::CreateFileVersionInfoForCurrentModule so that

base/file_version_info.h

Index: base/file_version_info.h
===================================================================
--- base/file_version_info.h	(revision 82561)
+++ base/file_version_info.h	(working copy)
@@ -6,11 +6,17 @@
 #define BASE_FILE_VERSION_INFO_H__
 #pragma once
 
+#include "build/build_config.h"
+#if defined(OS_WIN)
+#include <windows.h>
+// http://blogs.msdn.com/oldnewthing/archive/2004/10/25/247180.aspx
+extern "C" IMAGE_DOS_HEADER __ImageBase;
+#endif  // OS_WIN

[wtc, 2011/04/22 20:00:09]

Nit: the current Chromium Coding Style guide suggests
listing "build/build_config.h" among the regular headers,
and listing the platform-specific headers after the regular
headers:
http://dev.chromium.org/developers/coding-style#TOC-Platform-specific-code

[rvargas (doing something else), 2011/04/22 21:40:05]

Sure, but that doesn't cover the case of having to include <windows.h> itself.

+
 #include <string>
 
 #include "base/base_api.h"
 #include "base/string16.h"
-#include "build/build_config.h"
 
 class FilePath;
 
@@ -24,19 +30,34 @@
 // version returns values from the Info.plist as appropriate. TODO(avi): make
 // this a less-obvious Windows-ism.
 
-class BASE_API FileVersionInfo {
+class FileVersionInfo {
  public:
   virtual ~FileVersionInfo() {}
 #if defined(OS_WIN) || defined(OS_MACOSX)
   // Creates a FileVersionInfo for the specified path. Returns NULL if something
   // goes wrong (typically the file does not exit or cannot be opened). The
   // returned object should be deleted when you are done with it.
-  static FileVersionInfo* CreateFileVersionInfo(const FilePath& file_path);
+  BASE_API static FileVersionInfo* CreateFileVersionInfo(
+      const FilePath& file_path);
 #endif  // OS_WIN || OS_MACOSX
 
+#if defined(OS_WIN)
+  // Creates a FileVersionInfo for the specified module. Returns NULL in case
+  // of error. The returned object should be deleted when you are done with it.
+  BASE_API static FileVersionInfo* CreateFileVersionInfoForModule(
+      HMODULE module);
+
   // Creates a FileVersionInfo for the current module. Returns NULL in case
   // of error. The returned object should be deleted when you are done with it.
+  inline static FileVersionInfo* CreateFileVersionInfoForCurrentModule() {
+    HMODULE module = reinterpret_cast<HMODULE>(&__ImageBase);

[wtc, 2011/04/22 20:00:09]

What is __ImageBase?

[rvargas (doing something else), 2011/04/22 21:40:05]

Explained at line 12

+    return CreateFileVersionInfoForModule(module);
+  }

[wtc, 2011/04/22 20:00:09]

If we don't inline CreateFileVersionInfoForCurrentModule,
we won't need to declare and document
CreateFileVersionInfoForCurrentModule twice here.

But I guess this method MUST be inlined.  If so, please
document that.  Should we use the MSVC __forceinline keyword
instead?

[rvargas (doing something else), 2011/04/22 21:40:05]

Yes, the whole point is that we have to inline the method. We could use
__forceinline; I'm fine either way.

[brettw, 2011/04/22 21:43:34]

Force inline would make me feel better, and I think we should document this
clearly.

[rvargas (doing something else), 2011/04/22 22:06:43]

Done. Note that I'm not adding an additional guard for the compiler being
MSVS... the only other equivalent instance that I can find also assumes that
this is true for windows.

+#else
+  // Creates a FileVersionInfo for the current module. Returns NULL in case
+  // of error. The returned object should be deleted when you are done with it.
   static FileVersionInfo* CreateFileVersionInfoForCurrentModule();
+#endif  // OS_WIN
 
   // Accessors to the different version properties.
   // Returns an empty string if the property is not found.