0

mac/rlz: Remove an incorrect check

As a comment in the ScopedRlzValueStoreLock destructor explains:

  // Check that "store_ set" => "file_lock acquired". The converse isn't true,
  // for example if the rlz data file can't be read.

So don't CHECK when that happens, instead treat it like lock acquisition failures:
Silently drop events when that happens. I added a unit test for this scenario.

Also pass O_RDWR to open(), as posix requires one of O_READ, O_WRITE, or O_RDWR.

BUG=143950


Review URL: https://chromiumcodereview.appspot.com/10828424

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152659 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
thakis@chromium.org
2012-08-21 22:53:54 +00:00
parent ee914f9b49
commit 27e908b301
3 changed files with 35 additions and 5 deletions

@@ -20,6 +20,7 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "rlz/lib/rlz_lib.h" #include "rlz/lib/rlz_lib.h"
#include "rlz/lib/rlz_value_store.h"
#include "rlz/test/rlz_test_helpers.h" #include "rlz/test/rlz_test_helpers.h"
#if defined(OS_WIN) #if defined(OS_WIN)
@@ -871,4 +872,22 @@ TEST_F(RlzLibTest, ConcurrentStoreAccessWithProcessExitsWhileLockHeld) {
EXPECT_TRUE(rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER, EXPECT_TRUE(rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER,
rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL)); rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL));
} }
TEST_F(RlzLibTest, LockAcquistionSucceedsButPlistCannotBeCreated) {
// See the comment at the top of WriteFails.
if (!rlz_lib::SupplementaryBranding::GetBrand().empty())
return;
// Create a directory where the rlz file is supposed to appear. This way,
// the lock file can be created successfully, but creation of the rlz file
// itself will fail.
int mkdir_result = mkdir(rlz_lib::testing::RlzPlistFilenameStr().c_str(),
0500);
ASSERT_EQ(0, mkdir_result);
rlz_lib::SupplementaryBranding branding("TEST");
EXPECT_FALSE(rlz_lib::RecordProductEvent(rlz_lib::TOOLBAR_NOTIFIER,
rlz_lib::IE_DEFAULT_SEARCH, rlz_lib::INSTALL));
}
#endif #endif

@@ -107,6 +107,9 @@ class ScopedRlzValueStoreLock {
namespace testing { namespace testing {
// Prefix |directory| to the path where the RLZ data file lives, for tests. // Prefix |directory| to the path where the RLZ data file lives, for tests.
void SetRlzStoreDirectory(const FilePath& directory); void SetRlzStoreDirectory(const FilePath& directory);
// Returns the path of the plist file used as data store.
std::string RlzPlistFilenameStr();
} // namespace testing } // namespace testing
#endif // defined(OS_MACOSX) #endif // defined(OS_MACOSX)

@@ -270,7 +270,9 @@ bool RecursiveCrossProcessLock::TryGetCrossProcessLock(
const int kSleepPerTryMS = 200; const int kSleepPerTryMS = 200;
CHECK(file_lock_ == -1); CHECK(file_lock_ == -1);
file_lock_ = open([lock_filename fileSystemRepresentation], O_CREAT, 0666); file_lock_ = open([lock_filename fileSystemRepresentation],
O_RDWR | O_CREAT,
0666);
if (file_lock_ == -1) if (file_lock_ == -1)
return false; return false;
@@ -354,8 +356,8 @@ NSString* RlzPlistFilename() {
// Returns the path of the rlz lock file, also creates the parent directory // Returns the path of the rlz lock file, also creates the parent directory
// path if it doesn't exist. // path if it doesn't exist.
NSString* RlzLockFilename() { NSString* RlzLockFilename() {
NSString* const kRlzFile = @"flockfile"; NSString* const kRlzLockfile = @"flockfile";
return [CreateRlzDirectory() stringByAppendingPathComponent:kRlzFile]; return [CreateRlzDirectory() stringByAppendingPathComponent:kRlzLockfile];
} }
} // namespace } // namespace
@@ -377,8 +379,8 @@ ScopedRlzValueStoreLock::ScopedRlzValueStoreLock() {
} }
if (g_lock_depth > 1) { if (g_lock_depth > 1) {
// Reuse the already existing store object. // Reuse the already existing store object. Note that it can be NULL when
CHECK(g_store_object); // lock acquisition succeeded but the rlz data file couldn't be read.
store_.reset(g_store_object); store_.reset(g_store_object);
return; return;
} }
@@ -449,6 +451,12 @@ void SetRlzStoreDirectory(const FilePath& directory) {
} }
} }
std::string RlzPlistFilenameStr() {
@autoreleasepool {
return std::string([RlzPlistFilename() fileSystemRepresentation]);
}
}
} // namespace testing } // namespace testing
} // namespace rlz_lib } // namespace rlz_lib