Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(142)

Issue 2102363002: Revert of Don't use ::GetFileVersionInfo() in CreateFileVersionInfoForModule() (Closed)

Created:
4 years, 5 months ago by Pete Williamson
Modified:
4 years, 5 months ago
CC:
chromium-reviews, grt+watch_chromium.org, wfh+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Don't use ::GetFileVersionInfo() in CreateFileVersionInfoForModule() (patchset #13 id:240001 of https://codereview.chromium.org/2046583002/ ) Reason for revert: Sheriff reverting on suspicion of causing the Win x64 build to fail: https://build.chromium.org/p/chromium/builders/Win%20x64/builds/1995 From the log: FAILED: obj/base/base.file_version_info_win.obj ninja -t msvc -e environment.x64 -- C:\b\build\slave\cache\cipd\goma/gomacc "C:\b\depot_tools\win_toolchain\vs_files\95ddda401ec5678f15eeed01d2bee08fcbc5ee97\VC\bin\amd64\cl.exe" /nologo /showIncludes /FC @obj\base\base.file_version_info_win.obj.rsp /c ..\..\base\file_version_info_win.cc /Foobj\base\base.file_version_info_win.obj /Fdobj\base\base.cc.pdb c:\b\build\slave\win_x64\build\src\base\file_version_info_win.cc(81): error C2220: warning treated as error - no 'object' file generated c:\b\build\slave\win_x64\build\src\base\file_version_info_win.cc(81): warning C4267: 'argument': conversion from 'size_t' to 'DWORD', possible loss of data [8341/32887] CXX obj\base\threading\base.thread_collision_warner.obj Original issue's description: > Don't use ::GetFileVersionInfo() in CreateFileVersionInfoForModule() > > Currently, base::FileVersionInfo::CreateFileVersionInfoForModule() > calls ::GetModuleFileName and ::GetFileVersionInfo, grabs the loader > lock and potentially touches the disk to obtain the VS_VERSION_INFO > of the module. This is gratuitous for a module that is already loaded. > > With this CL, base::FileVersionInfo::CreateFileVersionInfoForModule() > uses base::win::GetResourceFromModule() to get the VS_VERSION_INFO > resource from memory. > > BUG=609709 > > Committed: https://crrev.com/f177a678814f97f0b35bd6aa678c1cf885ad1656 > Cr-Commit-Position: refs/heads/master@{#402549} TBR=grt@chromium.org,thestig@chromium.org,fdoray@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=609709 Committed: https://crrev.com/d916fc3b9ece7898b29b483a3cacd762f946ccf0 Cr-Commit-Position: refs/heads/master@{#402616}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+217 lines, -277 lines) Patch
M base/BUILD.gn View 2 chunks +3 lines, -1 line 0 comments Download
M base/base.gyp View 2 chunks +4 lines, -1 line 0 comments Download
A base/file_version_info_unittest.cc View 1 chunk +143 lines, -0 lines 0 comments Download
M base/file_version_info_win.h View 2 chunks +8 lines, -22 lines 0 comments Download
M base/file_version_info_win.cc View 4 chunks +48 lines, -73 lines 0 comments Download
D base/file_version_info_win_unittest.cc View 1 chunk +0 lines, -175 lines 0 comments Download
M chrome/browser/win/enumerate_modules_model.cc View 2 chunks +11 lines, -5 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
Pete Williamson
Created Revert of Don't use ::GetFileVersionInfo() in CreateFileVersionInfoForModule()
4 years, 5 months ago (2016-06-29 00:35:15 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2102363002/1
4 years, 5 months ago (2016-06-29 00:36:18 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 5 months ago (2016-06-29 00:37:55 UTC) #5
commit-bot: I haz the power
4 years, 5 months ago (2016-06-29 00:39:30 UTC) #7
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/d916fc3b9ece7898b29b483a3cacd762f946ccf0
Cr-Commit-Position: refs/heads/master@{#402616}

Powered by Google App Engine
This is Rietveld 408576698