Commit 7437f69a authored by Finnur Thorarinsson's avatar Finnur Thorarinsson Committed by Commit Bot

Revert "arcvm: Get the board name from the generated prop file instead"

This reverts commit 10646707.

Reason for revert: This is a speculative fix. Two Arc changelists
went into this build...
https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-rel/34691
... and I've already tried reverting the other one, but it didn't
fix the issue. 

If ArcAppDeferredLauncher* tests continue to fail with this revert
then this can be unreverted and the Arc tests need to be disabled.

Original change's description:
> arcvm: Get the board name from the generated prop file instead
> 
> On some boards like nami, CrOS rootfs' build.prop doesn't have the
> complete ro.product.board entry. It's sometimes a template. To always
> get the board name on all boards, ArcDefaultAppList needs to wait
> for ArcSessionManager to generate the board's own build.prop file
> in its stateful partition. This is the same as what arc-setup does
> today.
> 
> This CL also stops special casing ARCVM and does the same for
> ARC too. Also, when calling GetBoardName(), this CL uses
> MayBlock() to make it debug build compatible.
> 
> BUG=b:144199481
> TEST=try, ARCVM still starts
> 
> Change-Id: Ie878d9ffff98ecf3b9dccb837fd6dce1a382a4be
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026668
> Commit-Queue: Yusuke Sato <yusukes@chromium.org>
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#739250}

TBR=yusukes@chromium.org,khmel@google.com,khmel@chromium.org

Change-Id: I5a75922a9b101aafb1bb03de86826003a41ce599
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:144199481
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2044152Reviewed-by: default avatarFinnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739372}
parent 9f5f561e
...@@ -15,8 +15,6 @@ ...@@ -15,8 +15,6 @@
#include "base/strings/string_split.h" #include "base/strings/string_split.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/task/task_traits.h"
#include "base/task_runner.h"
#include "chrome/browser/chromeos/arc/arc_util.h" #include "chrome/browser/chromeos/arc/arc_util.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h" #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
...@@ -45,6 +43,10 @@ const base::FilePath::CharType kArcTestDirectory[] = ...@@ -45,6 +43,10 @@ const base::FilePath::CharType kArcTestDirectory[] =
FILE_PATH_LITERAL("arc_default_apps"); FILE_PATH_LITERAL("arc_default_apps");
const base::FilePath::CharType kArcTestBoardDirectory[] = const base::FilePath::CharType kArcTestBoardDirectory[] =
FILE_PATH_LITERAL("arc_board_default_apps"); FILE_PATH_LITERAL("arc_board_default_apps");
const base::FilePath::CharType kBoardDirectory[] =
FILE_PATH_LITERAL("/var/cache/arc_default_apps");
const base::FilePath::CharType kBuildProp[] =
FILE_PATH_LITERAL("/usr/share/arcvm/properties/build.prop");
bool use_test_apps_directory = false; bool use_test_apps_directory = false;
...@@ -175,42 +177,14 @@ ArcDefaultAppList::ArcDefaultAppList(Profile* profile, ...@@ -175,42 +177,14 @@ ArcDefaultAppList::ArcDefaultAppList(Profile* profile,
base::OnceClosure ready_callback) base::OnceClosure ready_callback)
: profile_(profile), ready_callback_(std::move(ready_callback)) { : profile_(profile), ready_callback_(std::move(ready_callback)) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
arc::ArcSessionManager::Get()->AddObserver(this);
}
ArcDefaultAppList::~ArcDefaultAppList() {
auto* manager = arc::ArcSessionManager::Get();
if (manager) // for unit testing
manager->RemoveObserver(this);
}
void ArcDefaultAppList::OnPropertyFilesExpanded(bool result) {
if (!result) {
// Failed to generate |kGeneratedPropertyFilesPath[Vm]| for whatever reason.
// Continue anyway not to stall the launcher initialization. In this case,
// ARC[VM] itself won't start, so not being able to get the board name won't
// be a huge problem either.
VLOG(1) << "Unable to get the board name.";
LoadDefaultApps(std::string());
return;
}
VLOG(1) << "Getting the board name";
const char* source_dir = arc::IsArcVmEnabled()
? arc::kGeneratedPropertyFilesPathVm
: arc::kGeneratedPropertyFilesPath;
base::PostTaskAndReplyWithResult(
FROM_HERE,
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_VISIBLE},
base::BindOnce(&GetBoardName,
base::FilePath(source_dir).Append("build.prop")),
base::BindOnce(&ArcDefaultAppList::LoadDefaultApps,
weak_ptr_factory_.GetWeakPtr()));
}
void ArcDefaultAppList::LoadDefaultApps(std::string board_name) { // Load default apps from two sources.
VLOG(1) << "Start loading default apps. Board name is " // /usr/share/google-chrome/extensions/arc - contains default apps for all
<< (board_name.empty() ? "<unknown>" : board_name); // boards that share the same image.
// /var/cache/arc_default_apps that is link to
// /usr/share/google-chrome/extensions/arc/BOARD_NAME - contains default
// apps for particular current board.
//
std::vector<base::FilePath> sources; std::vector<base::FilePath> sources;
base::FilePath base_path; base::FilePath base_path;
...@@ -220,8 +194,16 @@ void ArcDefaultAppList::LoadDefaultApps(std::string board_name) { ...@@ -220,8 +194,16 @@ void ArcDefaultAppList::LoadDefaultApps(std::string board_name) {
DCHECK(valid_path); DCHECK(valid_path);
const base::FilePath base_arc_path = base_path.Append(kArcDirectory); const base::FilePath base_arc_path = base_path.Append(kArcDirectory);
sources.push_back(base_arc_path); sources.push_back(base_arc_path);
if (!board_name.empty()) if (arc::IsArcVmEnabled()) {
sources.push_back(base_arc_path.Append(board_name)); // ARCVM environment doesn't have the symlink. Get the board name on the
// fly instead.
// TODO(yusukes): Do the same for ARC and remove the arc-setup code for
// creating the symlink.
sources.push_back(
base_arc_path.Append(GetBoardName(base::FilePath(kBuildProp))));
} else {
sources.push_back(base::FilePath(kBoardDirectory));
}
} else { } else {
const bool valid_path = const bool valid_path =
base::PathService::Get(chrome::DIR_TEST_DATA, &base_path); base::PathService::Get(chrome::DIR_TEST_DATA, &base_path);
...@@ -246,6 +228,8 @@ void ArcDefaultAppList::LoadDefaultApps(std::string board_name) { ...@@ -246,6 +228,8 @@ void ArcDefaultAppList::LoadDefaultApps(std::string board_name) {
} }
} }
ArcDefaultAppList::~ArcDefaultAppList() = default;
void ArcDefaultAppList::OnAppsRead(std::unique_ptr<AppInfoMap> apps) { void ArcDefaultAppList::OnAppsRead(std::unique_ptr<AppInfoMap> apps) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
class Profile; class Profile;
...@@ -26,7 +25,7 @@ class PrefRegistrySyncable; ...@@ -26,7 +25,7 @@ class PrefRegistrySyncable;
} // namespace user_prefs } // namespace user_prefs
// Contains map of default pre-installed apps and packages. // Contains map of default pre-installed apps and packages.
class ArcDefaultAppList : public arc::ArcSessionManager::Observer { class ArcDefaultAppList {
public: public:
struct AppInfo { struct AppInfo {
AppInfo(const std::string& name, AppInfo(const std::string& name,
...@@ -58,7 +57,7 @@ class ArcDefaultAppList : public arc::ArcSessionManager::Observer { ...@@ -58,7 +57,7 @@ class ArcDefaultAppList : public arc::ArcSessionManager::Observer {
using AppInfoMap = std::map<std::string, std::unique_ptr<AppInfo>>; using AppInfoMap = std::map<std::string, std::unique_ptr<AppInfo>>;
ArcDefaultAppList(Profile* profile, base::OnceClosure ready_callback); ArcDefaultAppList(Profile* profile, base::OnceClosure ready_callback);
~ArcDefaultAppList() override; ~ArcDefaultAppList();
static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry); static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
static void UseTestAppsDirectory(); static void UseTestAppsDirectory();
...@@ -91,18 +90,6 @@ class ArcDefaultAppList : public arc::ArcSessionManager::Observer { ...@@ -91,18 +90,6 @@ class ArcDefaultAppList : public arc::ArcSessionManager::Observer {
} }
private: private:
// arc::ArcSessionManager::Observer:
void OnPropertyFilesExpanded(bool result) override;
// Loads default apps from two sources:
//
// /usr/share/google-chrome/extensions/arc - contains default apps for all
// boards that share the same image.
// /usr/share/google-chrome/extensions/arc/<BOARD_NAME> - contains default
// apps for particular current board.
//
void LoadDefaultApps(std::string board_name);
// Called when default apps are read from the provided source. // Called when default apps are read from the provided source.
void OnAppsRead(std::unique_ptr<AppInfoMap> apps); void OnAppsRead(std::unique_ptr<AppInfoMap> apps);
// Called when default apps from all sources are read. // Called when default apps from all sources are read.
......
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