Commit 0acc75bf authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

Revert "Chromad: Fix kerberos files tests"

This reverts commit 7a20ffc4.

Reason for revert: Still flaky
https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyUwsSBUZsYWtlIkhFeGlzdGluZ1VzZXJDb250cm9sbGVyQWN0aXZlRGlyZWN0b3J5VGVzdC5Qb2xpY3lDaGFuZ2VUcmlnZ2Vyc0ZpbGVVcGRhdGUM

Original change's description:
> Chromad: Fix kerberos files tests
>
> Modify KerberosFilesChangeWaiter to be more robust callbacks called
> several times
>
> BUG=chromium:865206
>
> Change-Id: I0c5f22fb26871607257d3b29ad3bd88d5b505c7c
> Reviewed-on: https://chromium-review.googlesource.com/1145074
> Commit-Queue: Alexander Alekseev <alemate@chromium.org>
> Reviewed-by: Alexander Alekseev <alemate@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576941}

TBR=alemate@chromium.org,rsorokin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 865206, 867386
Change-Id: I97bc01ed73dcd0dcc7c0fa3735f40548edc92a81
Reviewed-on: https://chromium-review.googlesource.com/1150061
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarYoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578207}
parent b9a893cd
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
// found in the LICENSE file. // found in the LICENSE file.
#include <string> #include <string>
#include <unordered_set>
#include <vector> #include <vector>
#include "base/barrier_closure.h" #include "base/barrier_closure.h"
...@@ -139,6 +138,16 @@ class KerberosFilesChangeWaiter { ...@@ -139,6 +138,16 @@ class KerberosFilesChangeWaiter {
// If |files_must_exist| is true and a file already exists the class does not // If |files_must_exist| is true and a file already exists the class does not
// wait when it changes. // wait when it changes.
explicit KerberosFilesChangeWaiter(bool files_must_exist) { explicit KerberosFilesChangeWaiter(bool files_must_exist) {
barrier_closure_ = base::BarrierClosure(2, loop_.QuitClosure());
watch_callback_ = base::BindRepeating(
[](const base::RepeatingClosure& barrier_closure,
const base::FilePath& path, bool error) -> void {
EXPECT_FALSE(error);
barrier_closure.Run();
},
barrier_closure_);
config_watcher_ = std::make_unique<base::FilePathWatcher>(); config_watcher_ = std::make_unique<base::FilePathWatcher>();
MaybeStartWatch(&config_watcher_, MaybeStartWatch(&config_watcher_,
base::FilePath(GetKerberosConfigFileName()), base::FilePath(GetKerberosConfigFileName()),
...@@ -148,8 +157,6 @@ class KerberosFilesChangeWaiter { ...@@ -148,8 +157,6 @@ class KerberosFilesChangeWaiter {
MaybeStartWatch(&creds_watcher_, MaybeStartWatch(&creds_watcher_,
base::FilePath(GetKerberosCredentialsCacheFileName()), base::FilePath(GetKerberosCredentialsCacheFileName()),
files_must_exist); files_must_exist);
if (watching_paths_.empty())
loop_.Quit();
} }
// Should be called once. // Should be called once.
...@@ -163,31 +170,19 @@ class KerberosFilesChangeWaiter { ...@@ -163,31 +170,19 @@ class KerberosFilesChangeWaiter {
void MaybeStartWatch(std::unique_ptr<base::FilePathWatcher>* watcher, void MaybeStartWatch(std::unique_ptr<base::FilePathWatcher>* watcher,
const base::FilePath& path, const base::FilePath& path,
bool files_must_exist) { bool files_must_exist) {
(*watcher)->Watch( (*watcher)->Watch(path, false /* recursive */, watch_callback_);
path, false /* recursive */,
base::BindRepeating(&KerberosFilesChangeWaiter::WatchCallback,
weak_factory_.GetWeakPtr()));
if (!files_must_exist && base::PathExists(path)) { if (!files_must_exist && base::PathExists(path)) {
// File was written so stop the watch. watch_callback_.Run(path, false /* error */);
watcher->reset(); watcher->reset();
return;
} }
watching_paths_.insert(path.value());
}
void WatchCallback(const base::FilePath& path, bool error) {
EXPECT_FALSE(error);
watching_paths_.erase(path.value());
if (watching_paths_.empty())
loop_.Quit();
} }
base::RunLoop loop_; base::RunLoop loop_;
std::unordered_set<std::string> watching_paths_; base::RepeatingClosure barrier_closure_;
base::RepeatingCallback<void(const base::FilePath& path, bool error)>
watch_callback_;
std::unique_ptr<base::FilePathWatcher> config_watcher_; std::unique_ptr<base::FilePathWatcher> config_watcher_;
std::unique_ptr<base::FilePathWatcher> creds_watcher_; std::unique_ptr<base::FilePathWatcher> creds_watcher_;
base::WeakPtrFactory<KerberosFilesChangeWaiter> weak_factory_{this};
}; };
} // namespace } // namespace
...@@ -982,7 +977,7 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest, ...@@ -982,7 +977,7 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
// Kerberos files. // Kerberos files.
// Disabled due to flakiness, see https://crbug.com/865206. // Disabled due to flakiness, see https://crbug.com/865206.
IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest, IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
PolicyChangeTriggersFileUpdate) { DISABLED_PolicyChangeTriggersFileUpdate) {
LoginAdOnline(); LoginAdOnline();
ApplyPolicyAndWaitFilesChanged(false /* enable_dns_cname_lookup */); ApplyPolicyAndWaitFilesChanged(false /* enable_dns_cname_lookup */);
...@@ -995,8 +990,9 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest, ...@@ -995,8 +990,9 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest,
// Tests if user Kerberos files changed D-Bus signal triggers updating user // Tests if user Kerberos files changed D-Bus signal triggers updating user
// Kerberos files. // Kerberos files.
// Disabled due to flakiness, see https://crbug.com/865206. // Disabled due to flakiness, see https://crbug.com/865206.
IN_PROC_BROWSER_TEST_F(ExistingUserControllerActiveDirectoryTest, IN_PROC_BROWSER_TEST_F(
UserKerberosFilesChangedSignalTriggersFileUpdate) { ExistingUserControllerActiveDirectoryTest,
DISABLED_UserKerberosFilesChangedSignalTriggersFileUpdate) {
LoginAdOnline(); LoginAdOnline();
KerberosFilesChangeWaiter files_change_waiter(true /* files_must_exist */); KerberosFilesChangeWaiter files_change_waiter(true /* files_must_exist */);
fake_authpolicy_client()->SetUserKerberosFiles("new_kerberos_creds", fake_authpolicy_client()->SetUserKerberosFiles("new_kerberos_creds",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment