Skip to content

Commit

Permalink
Use brave-style user dir on linux
Browse files Browse the repository at this point in the history
Brave support four different user dir depends on channel.
* Brave-Browser for stable release
* Brave-Browser-Beta for beta release
* Brave-Browser-Dev for dev release
* Brave-Development for unofficial build

Canary channel isn't supported on linux.

Channel suffixes are determined from $CHROME_VERSION_EXTRA, which is
passed by the launch wrapper script.
For more detail, see src/docs/user_data_dir.md#Linux
  • Loading branch information
simonhong committed Apr 11, 2018
1 parent 702f616 commit 6c0e4c4
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 14 deletions.
26 changes: 26 additions & 0 deletions common/brave_channel_info_linux.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/common/brave_channel_info_linux.h"

namespace brave {

std::string GetChannelSuffixForDataDir() {
std::string modifier;
std::string data_dir_suffix;

char* env = getenv("CHROME_VERSION_EXTRA");
if (env)
modifier = env;

// Chrome doesn't support canary channel on linux.
if (modifier == "unstable") // linux version of "dev"
data_dir_suffix = "-Dev";
else if (modifier == "beta")
data_dir_suffix = "-Beta";

return data_dir_suffix;
}

} // namespace brave
16 changes: 16 additions & 0 deletions common/brave_channel_info_linux.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMMON_BRAVE_CHANNEL_INFO_LINUX_H_
#define BRAVE_COMMON_BRAVE_CHANNEL_INFO_LINUX_H_

#include <string>

namespace brave {

std::string GetChannelSuffixForDataDir();

} // namespace brave

#endif // BRAVE_COMMON_BRAVE_CHANNEL_INFO_LINUX_H_
16 changes: 8 additions & 8 deletions patches/chrome-common-BUILD.gn.patch
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/chrome/common/BUILD.gn b/chrome/common/BUILD.gn
index d08a945c6b11a918fa5faedcb1c87c269772e877..946a111464968bf780a4cf9ffad851c435b4e228 100644
index d08a945c6b11a918fa5faedcb1c87c269772e877..7c1daac9f4ff71120ba2eab713e5aec1e24c4bf1 100644
--- a/chrome/common/BUILD.gn
+++ b/chrome/common/BUILD.gn
@@ -528,6 +528,13 @@ static_library("non_code_constants") {
Expand All @@ -20,13 +20,13 @@ index d08a945c6b11a918fa5faedcb1c87c269772e877..946a111464968bf780a4cf9ffad851c4
"pref_names.h",
]

+ if (brave_chromium_build) {
+ if (is_mac) {
+ sources += [
+ "//brave/common/brave_paths_mac.mm",
+ "//brave/common/brave_paths_mac.h",
+ ]
+ }
+ if (brave_chromium_build && is_official_build) {
+ sources += [
+ "//brave/common/brave_paths_mac.mm",
+ "//brave/common/brave_paths_mac.h",
+ "//brave/common/brave_channel_info_linux.cc",
+ "//brave/common/brave_channel_info_linux.h",
+ ]
+ }
+
public_deps = [
Expand Down
30 changes: 24 additions & 6 deletions patches/chrome-common-chrome_paths_linux.cc.patch
Original file line number Diff line number Diff line change
@@ -1,16 +1,34 @@
diff --git a/chrome/common/chrome_paths_linux.cc b/chrome/common/chrome_paths_linux.cc
index fc47bd3f12418fdaed879e0b485bfc7cb572a6e8..b646812c6abafc22ffe62e06e46d818e15c80984 100644
index fc47bd3f12418fdaed879e0b485bfc7cb572a6e8..9d7f74be49456b05c593ae2df4831c754d931a73 100644
--- a/chrome/common/chrome_paths_linux.cc
+++ b/chrome/common/chrome_paths_linux.cc
@@ -89,8 +89,10 @@ bool GetDefaultUserDataDirectory(base::FilePath* result) {
@@ -16,6 +16,10 @@
#include "chrome/common/channel_info.h"
#include "chrome/common/chrome_paths_internal.h"

+#if defined(BRAVE_CHROMIUM_BUILD) && defined(OFFICIAL_BUILD)
+#include "brave/common/brave_channel_info_linux.h"
+#endif
+
namespace chrome {

using base::nix::GetXDGDirectory;
@@ -87,10 +91,18 @@ bool GetDefaultUserDataDirectory(base::FilePath* result) {
GetXDGDirectory(env.get(), kXdgConfigHomeEnvVar, kDotConfigDir);
}

+#if defined(BRAVE_CHROMIUM_BUILD)
+#if defined(OFFICIAL_BUILD)
+ *result = config_dir.Append("Brave-Browser" + brave::GetChannelSuffixForDataDir());
+#else
+ *result = config_dir.Append("Brave-Browser-Development");
+#endif
+#else
#if defined(GOOGLE_CHROME_BUILD)
*result = config_dir.Append("google-chrome" + GetChannelSuffixForDataDir());
+#elif defined(OFFICIAL_BUILD)
+ *result = config_dir.Append("brave-browser");
#else
- *result = config_dir.Append("chromium");
+ *result = config_dir.Append("brave-browser-development");
*result = config_dir.Append("chromium");
+#endif
#endif
return true;
}

8 comments on commit 6c0e4c4

@bridiver
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't going to work correctly if CHROME_CONFIG_HOME is set

@bridiver
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as suggested #213 we should be using a method override instead of a patch. That will remove the CHROME_CONFIG_HOME issue

@simonhong
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If CHROME_CONFIG_HOME is set, it can act as a parent directory of our directory.
If we don't want that option, this can be removed by overriding.

@bridiver
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think the problem is that someone may have it set for Chrome and then Brave will go in there as well. That may or may not be the user's intent so I think it's best to override here

@bridiver
Copy link
Collaborator

@bridiver bridiver commented on 6c0e4c4 Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also merge the OFFICIAL_BUILD -Development into GetChannelSuffixForDataDir? I can't see any reason to handle them separately

@simonhong
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@simonhong
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also merge the OFFICIAL_BUILD -Development into GetChannelSuffixForDataDir? I can't see any reason to handle them separately

#205 cleaned up it. PTAL this issue, too.

@bridiver
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the official build change, but we're still patching instead of overriding the method in chromium_src

Please sign in to comment.