Commit 8b061784 authored by Xianzhu Wang's avatar Xianzhu Wang

Revert "Query Tiles: Use TileIterator to implement TileManager::GetTile()."

This reverts commit f47c5813.

Reason for revert: The new test is failing on multiple bots, e.g.
https://ci.chromium.org/p/chromium/builders/ci/Mac10.13%20Tests/24417
https://ci.chromium.org/p/chromium/builders/ci/Mac10.10%20Tests/53585

Original change's description:
> Query Tiles: Use TileIterator to implement TileManager::GetTile().
> 
> Replace the recursive implementation with TileItartor. Also test is
> polished to cover the empty return case.
> 
> Bug: NONE
> Change-Id: Ifd86c560a89f639d4920cff88df5cca044fa83cb
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2177297
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Reviewed-by: Hesen Zhang <hesen@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#765329}

TBR=dtrainor@chromium.org,qinmin@chromium.org,shaktisahu@chromium.org,xingliu@chromium.org,hesen@chromium.org

Change-Id: I2acfc7c83b151d841dac914bb7b8195da63abdfa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: NONE
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2181052Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#765373}
parent 1896b056
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/task_runner.h" #include "base/task_runner.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/query_tiles/internal/tile_iterator.h"
#include "components/query_tiles/internal/tile_manager.h" #include "components/query_tiles/internal/tile_manager.h"
namespace upboarding { namespace upboarding {
...@@ -68,17 +67,32 @@ class TileManagerImpl : public TileManager { ...@@ -68,17 +67,32 @@ class TileManagerImpl : public TileManager {
FROM_HERE, base::BindOnce(std::move(callback), std::move(tiles))); FROM_HERE, base::BindOnce(std::move(callback), std::move(tiles)));
} }
Tile* FindTile(const std::string& tile_id, Tile* root) {
if (tile_id == root->id)
return root;
for (auto& child : root->sub_tiles) {
if (child->id == tile_id)
return child.get();
}
Tile* result = nullptr;
for (auto& child : root->sub_tiles) {
result = FindTile(tile_id, child.get());
if (result)
return result;
}
return nullptr;
}
void GetTile(const std::string& tile_id, TileCallback callback) override { void GetTile(const std::string& tile_id, TileCallback callback) override {
const Tile* result = nullptr; Tile* result = nullptr;
if (tile_group_ && ValidateGroup(tile_group_.get())) { if (tile_group_ && ValidateGroup(tile_group_.get())) {
TileIterator it(*tile_group_, TileIterator::kAllTiles); for (const auto& tile : tile_group_->tiles) {
while (it.HasNext()) { result = FindTile(tile_id, tile.get());
const auto* tile = it.Next(); if (result)
DCHECK(tile);
if (tile->id == tile_id) {
result = tile;
break; break;
}
} }
} }
......
...@@ -60,29 +60,6 @@ class TileManagerTest : public testing::Test { ...@@ -60,29 +60,6 @@ class TileManagerTest : public testing::Test {
loop.Run(); loop.Run();
} }
// TODO(crbug.com/1078163): Replace Init() with InitWithData.
void InitWithData(TileGroupStatus expected_status,
std::vector<TileGroup> groups,
bool success = true) {
MockTileStore::KeysAndEntries entries;
for (const auto& group : groups) {
entries[group.id] = std::make_unique<TileGroup>(group);
}
EXPECT_CALL(*tile_store(), InitAndLoad(_))
.WillOnce(Invoke(
[&](base::OnceCallback<void(bool, MockTileStore::KeysAndEntries)>
callback) {
std::move(callback).Run(success, std::move(entries));
}));
base::RunLoop loop;
manager()->Init(base::BindOnce(&TileManagerTest::OnInitCompleted,
base::Unretained(this), loop.QuitClosure(),
expected_status));
loop.Run();
}
void OnInitCompleted(base::RepeatingClosure closure, void OnInitCompleted(base::RepeatingClosure closure,
TileGroupStatus expected_status, TileGroupStatus expected_status,
TileGroupStatus status) { TileGroupStatus status) {
...@@ -126,7 +103,7 @@ class TileManagerTest : public testing::Test { ...@@ -126,7 +103,7 @@ class TileManagerTest : public testing::Test {
std::move(closure).Run(); std::move(closure).Run();
} }
void GetSingleTile(const std::string& id, base::Optional<Tile> expected) { void GetSingleTile(const std::string& id, Tile expected) {
base::RunLoop loop; base::RunLoop loop;
manager()->GetTile( manager()->GetTile(
id, base::BindOnce(&TileManagerTest::OnGetTile, base::Unretained(this), id, base::BindOnce(&TileManagerTest::OnGetTile, base::Unretained(this),
...@@ -137,9 +114,7 @@ class TileManagerTest : public testing::Test { ...@@ -137,9 +114,7 @@ class TileManagerTest : public testing::Test {
void OnGetTile(base::RepeatingClosure closure, void OnGetTile(base::RepeatingClosure closure,
base::Optional<Tile> expected, base::Optional<Tile> expected,
base::Optional<Tile> actual) { base::Optional<Tile> actual) {
ASSERT_EQ(expected.has_value(), actual.has_value()); EXPECT_TRUE(test::AreTilesIdentical(expected.value(), actual.value()));
if (expected.has_value())
EXPECT_TRUE(test::AreTilesIdentical(expected.value(), actual.value()));
std::move(closure).Run(); std::move(closure).Run();
} }
...@@ -243,6 +218,7 @@ TEST_F(TileManagerTest, InitAndLoadSuccess) { ...@@ -243,6 +218,7 @@ TEST_F(TileManagerTest, InitAndLoadSuccess) {
Init(TileGroupStatus::kSuccess); Init(TileGroupStatus::kSuccess);
GetTiles(expected); GetTiles(expected);
GetSingleTile("guid-1-1", expected[0]);
} }
// Failed to init an empty db, and save tiles call failed because of db is // Failed to init an empty db, and save tiles call failed because of db is
...@@ -366,15 +342,6 @@ TEST_F(TileManagerTest, SaveTilesAndReplaceOldGroupSuccess) { ...@@ -366,15 +342,6 @@ TEST_F(TileManagerTest, SaveTilesAndReplaceOldGroupSuccess) {
GetTiles(std::move(expected)); GetTiles(std::move(expected));
} }
// Verifies GetTile(tile_id) API can return the right thing.
TEST_F(TileManagerTest, GetTileById) {
TileGroup group;
test::ResetTestGroup(&group);
InitWithData(TileGroupStatus::kSuccess, {group});
GetSingleTile("guid-1-1", *group.tiles[0]);
GetSingleTile("id_not_exist", base::nullopt);
}
} // namespace } // namespace
} // namespace upboarding } // namespace upboarding
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