Linux support #205

simonhong · 2018-06-29T05:43:08Z

Use brave app icon on all channel.
Add nightly channel support on linux
Cleanup channel info related code

Issue: brave/brave-browser#10
Close brave/brave-browser#340
Close brave/brave-browser#461

TODO: image files should be checked by design team.

Submitter Checklist:

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).
Tagged reviewers and labelled the pull request as needed.
Request a security/privacy review as needed.

Test Plan:

yarn test brave_browser_tests --filter=BraveResourcesBrowserTest.*
yarn test brave_browser_tests --filter=BraveViewsDelegateLinuxBrowserTest.*
yarn test brave_unit_tests --filter=BraveChannelInfoTest.*

Reviewer Checklist:

New files have MPL-2.0 license header.
Request a security/privacy review as needed.
Adequate test coverage exists to prevent regressions

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

bridiver · 2018-07-02T23:10:11Z

patches/chrome-common-chrome_paths_linux.cc.patch

 +#include "brave/common/brave_channel_info_linux.h"
 +#endif
 +
 namespace chrome {

 using base::nix::GetXDGDirectory;
-@@ -87,10 +91,18 @@ bool GetDefaultUserDataDirectory(base::FilePath* result) {
+@@ -87,10 +91,14 @@ bool GetDefaultUserDataDirectory(base::FilePath* result) {


can we do a method override here instead of patch as discussed?

bridiver · 2018-07-02T23:45:52Z

common/brave_channel_info_linux.cc

@@ -16,11 +17,18 @@ std::string GetChannelSuffixForDataDir() {

  // Chrome doesn't support canary channel on linux.
  if (modifier == "unstable")  // linux version of "dev"
+    modifier = "dev";


if we use defines in a gni config as suggested https://github.com/brave/brave-core/pull/208/files#r199649672 we can remove some or all of this logic and just return the value from the gn define

otherwise I'm concerned about the two getting out of sync

There are many unit test and browser test that uses this modifier.
So, if someone make inconsistency between c++ and gn, it can be easily detected. WDYT?

Unified all linux channel infos to brave/build/linux and used it in c++.
PTAL.

simonhong · 2018-07-04T03:09:40Z

Sorry, additional nightly channel support commit is added to this PR because nightly supports depends on this PR.
So, this PR's title is changed to a little bit general.
Please check each commit's message.

simonhong · 2018-07-04T03:14:10Z

I think preparing nightly build release is almost done with this PR.

Set parest user dir - .../BraveSoftware/Brave-Browser/... Added unit test - BraveChannelInfoTest

Channel determining logic is unified to brave::GetChannelImpl().

…Linux They are subclass of ChromeViewsDelegate and ChromeBrowserMainExtraPartsViewsLinux. They are introduced to override ChromeViewsDelegate::GetWindowIcon() in chrome_views_delegate_linux.cc.

BraveViewsDelegateLinuxBrowserTest.GetDefaultWindowIconTest checks whether proper image is returned from ViewsDelegate::GetDefaultWindowIcon()

darkdh · 2018-07-05T17:36:44Z

build/linux/channels.gni

+linux_channel_stable = "stable"
+linux_channel_beta = "beta"
+linux_channel_dev = "unstable"
+linux_channel_nightly = "nightly"


can we change this to brave_linux_channel_nightly which is not in upstream?

darkdh · 2018-07-05T17:37:32Z

build/linux/channels.gni

+linux_channel_nightly = "nightly"
+
+# Our channel name and upstream linux channel name is different.
+linux_channel_name = ""


what is the purpose of this line?

Oops. forgot to delete. done.

Define channel in brave/build/linux/BUILD.gn and use it in C++ world.

darkdh

lgtm

bridiver · 2018-07-05T23:06:29Z

patches/chrome-browser-chrome_content_browser_client.cc.patch

+ #include "chrome/services/file_util/public/mojom/constants.mojom.h"
+ #endif
+
+#if defined(BRAVE_CHROMIUM_BUILD) && defined(OS_LINUX)


@simonhong can you remove these patches in a follow-up? The chromium_src subclass override I mentioned on slack would work well here

in fact in this case you subclass the ViewsDelegateLinux and you won't even need BraveBrowserMainExtraPartsViewLinux

Done in #230

bridiver · 2018-07-05T23:10:04Z

patches/chrome-installer-linux-BUILD.gn.patch

+ linux_package("unstable") {
+   channel = "unstable"
+ }
+linux_package("nightly") {


can this be added in a brave build file or does it have to be here?

To add this in a brave build, we need to copy linux_package template in here to us.
So, I added this here to avoid copy.

bridiver · 2018-07-05T23:10:37Z

patches/chrome-installer-linux-rpm-build.sh.patch

@@ -25,3 +25,13 @@ index 261faeaf26f34f2e89229579ae7933e23e4299e6..201aacd532ea70067279ec970134f3b2
   mv "$RPMBUILD_DIR/RPMS/$ARCHITECTURE/${PKGNAME}.${ARCHITECTURE}.rpm" \
      "${OUTPUTDIR}"
   # Make sure the package is world-readable, otherwise it causes problems when
+@@ -146,6 +150,9 @@ verify_channel() {
+     unstable|dev|alpha )
+       CHANNEL=unstable


actually can we use unstable as nightly or do we need both?

We need both - linux uses unstable and dev both for dev channel.

disable nacl

simonhong self-assigned this Jun 29, 2018

simonhong requested a review from darkdh June 29, 2018 05:43

simonhong changed the title ~~Apply brave's beta and dev app icon in official build~~ Apply brave's beta and dev app icon in linux official build Jun 29, 2018

simonhong force-pushed the app_icon_linux branch from b928e50 to 69884eb Compare June 30, 2018 04:15

simonhong changed the title ~~Apply brave's beta and dev app icon in linux official build~~ Change app icon and cleanup channel info code on linux Jun 30, 2018

simonhong force-pushed the app_icon_linux branch from 69884eb to 91bb69b Compare June 30, 2018 05:27

simonhong requested a review from bridiver July 2, 2018 23:09

bridiver reviewed Jul 2, 2018

View reviewed changes

simonhong force-pushed the app_icon_linux branch 3 times, most recently from 13dd130 to 0486495 Compare July 3, 2018 07:19

simonhong mentioned this pull request Jul 3, 2018

Prepare for dev release brave/brave-browser#396

Closed

12 tasks

simonhong changed the title ~~Change app icon and cleanup channel info code on linux~~ WIP: Change app icon and cleanup channel info code on linux Jul 3, 2018

simonhong added the pr/work-in-progress label Jul 3, 2018

simonhong force-pushed the app_icon_linux branch 2 times, most recently from cf3b194 to 031183a Compare July 3, 2018 14:26

simonhong changed the title ~~WIP: Change app icon and cleanup channel info code on linux~~ Change app icon and cleanup channel info code on linux Jul 4, 2018

simonhong removed the pr/work-in-progress label Jul 4, 2018

simonhong force-pushed the app_icon_linux branch 3 times, most recently from 5de783a to 5e65f49 Compare July 4, 2018 03:02

simonhong changed the title ~~Change app icon and cleanup channel info code on linux~~ Linux support Jul 4, 2018

simonhong force-pushed the app_icon_linux branch from 9807777 to d40f6ed Compare July 5, 2018 04:56

simonhong added 3 commits July 5, 2018 13:57

Apply brave's beta and dev app icon in official build

e791ca3

Cleanup channel info code in linux and add some tests

02c7b18

Set parest user dir - .../BraveSoftware/Brave-Browser/... Added unit test - BraveChannelInfoTest

Override GetDefaultUserDataDirectory() in chrome_paths_linux.cc

8ed6d4c

simonhong added 5 commits July 5, 2018 13:57

Copying channel_info_posix.cc to unify channel logic in one place

e0be092

Channel determining logic is unified to brave::GetChannelImpl().

Introduce BraveViewsDelegateLInux and BraveBrowserMainExtraPartsViews…

d5d529c

…Linux They are subclass of ChromeViewsDelegate and ChromeBrowserMainExtraPartsViewsLinux. They are introduced to override ChromeViewsDelegate::GetWindowIcon() in chrome_views_delegate_linux.cc.

Use brave's icon for development build

1b715fd

Add browser test for BraveViewsDelegateLinux

404c84a

BraveViewsDelegateLinuxBrowserTest.GetDefaultWindowIconTest checks whether proper image is returned from ViewsDelegate::GetDefaultWindowIcon()

Support nightly channel on Linux

980fda8

simonhong force-pushed the app_icon_linux branch from d40f6ed to da4adb2 Compare July 5, 2018 04:58

darkdh reviewed Jul 5, 2018

View reviewed changes

Unify linux channel string info to brave/build/linux/channels.gni

a65617b

Define channel in brave/build/linux/BUILD.gn and use it in C++ world.

simonhong force-pushed the app_icon_linux branch from da4adb2 to a65617b Compare July 5, 2018 19:59

darkdh approved these changes Jul 5, 2018

View reviewed changes

simonhong merged commit 6cd4cdf into master Jul 5, 2018

simonhong deleted the app_icon_linux branch July 5, 2018 21:58

bridiver reviewed Jul 5, 2018

View reviewed changes

fmarier pushed a commit that referenced this pull request Oct 29, 2019

Merge pull request #205 from /issues/154

1f567a5

disable nacl

petemill pushed a commit that referenced this pull request Jul 27, 2020

Merge pull request #205 from /issues/154

231d770

disable nacl

petemill pushed a commit that referenced this pull request Jul 28, 2020

Merge pull request #205 from /issues/154

abdcef0

disable nacl

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux support #205

Linux support #205

simonhong commented Jun 29, 2018 •

edited

Loading

bridiver Jul 2, 2018

simonhong Jul 3, 2018

bridiver Jul 2, 2018

bridiver Jul 2, 2018

simonhong Jul 4, 2018

simonhong Jul 5, 2018

simonhong commented Jul 4, 2018 •

edited

Loading

simonhong commented Jul 4, 2018

darkdh Jul 5, 2018

simonhong Jul 5, 2018

darkdh Jul 5, 2018

simonhong Jul 5, 2018

darkdh left a comment

bridiver Jul 5, 2018

bridiver Jul 5, 2018

simonhong Jul 7, 2018

bridiver Jul 5, 2018

simonhong Jul 7, 2018

bridiver Jul 5, 2018

simonhong Jul 7, 2018

Linux support #205

Linux support #205

Conversation

simonhong commented Jun 29, 2018 • edited Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonhong commented Jul 4, 2018 • edited Loading

simonhong commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

darkdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonhong commented Jun 29, 2018 •

edited

Loading

simonhong commented Jul 4, 2018 •

edited

Loading