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

Issue 1101723004: Revert "Intercept base::File Open/Close" (Closed)

Created:
5 years, 8 months ago by pasko
Modified:
5 years, 8 months ago
Reviewers:
Nico, brettw
CC:
chromium-reviews, erikwright+watch_chromium.org, Sergey Ulanov, gavinp
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert "Intercept base::File Open/Close" This reverts commit 45a0dc0b75b52e026eb15526ef441edc5dbe9ba5. > Intercept base::File Open/Close > > When a file descriptor is opened by the base::File, all calls to close(3) from > the same dynamic library will hit a CHECK unless they are made from a > whitelist of callsites belonging to base::File. > > There is a handy protect_file_posix.gypi introduced to make it easy to enable > on Chrome-for-Android. > > This 'linker magic' is somewhat crazy, so: > 1. it will be *removed *when crbug.com/424562 is fixed > 2. it should only be used by a whitelist of binaries/libraries (in the > opensource part: libchromeshell only) > > BUG=424562 > > Review URL: https://codereview.chromium.org/676873004 > > Cr-Commit-Position: refs/heads/master@{#304592} Reason: crashes are not numerous, not much sense to fix, some explanations found elsewhere. BUG=424562 Committed: https://crrev.com/e1ceecf5545568261956de95fda59270a91d4ef0 Cr-Commit-Position: refs/heads/master@{#327051}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -238 lines) Patch
M BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M base/BUILD.gn View 1 chunk +0 lines, -11 lines 0 comments Download
M base/base.gyp View 1 chunk +0 lines, -18 lines 0 comments Download
M base/files/file.cc View 3 chunks +0 lines, -10 lines 0 comments Download
M base/files/file_posix.cc View 6 chunks +1 line, -16 lines 0 comments Download
D base/files/file_posix_hooks_internal.h View 1 chunk +0 lines, -31 lines 0 comments Download
D base/files/protect_file_posix.cc View 1 chunk +0 lines, -106 lines 0 comments Download
D base/files/protect_file_posix.gypi View 1 chunk +0 lines, -31 lines 0 comments Download
M build/gn_migration.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/android/BUILD.gn View 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/chrome_shell.gypi View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 14 (5 generated)
pasko
Finally reverting the hack, please take a look.
5 years, 8 months ago (2015-04-24 19:14:30 UTC) #2
Nico
lgtm, thanks!
5 years, 8 months ago (2015-04-24 19:57:10 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1101723004/1
5 years, 8 months ago (2015-04-27 09:11:25 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/59181)
5 years, 8 months ago (2015-04-27 09:17:03 UTC) #7
pasko
brettw: please review the one line change in BUILD.gn
5 years, 8 months ago (2015-04-27 10:52:04 UTC) #9
brettw
lgtm
5 years, 8 months ago (2015-04-27 15:36:40 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1101723004/1
5 years, 8 months ago (2015-04-27 15:51:59 UTC) #12
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-04-27 15:55:24 UTC) #13
commit-bot: I haz the power
5 years, 8 months ago (2015-04-27 15:56:25 UTC) #14
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/e1ceecf5545568261956de95fda59270a91d4ef0
Cr-Commit-Position: refs/heads/master@{#327051}

Powered by Google App Engine
This is Rietveld 408576698