Try adding assert(toolkit_views) to extensions/BUILD.gn

[sangwoo108]

Extensions component isn't enabled on Android. So we might be able to add assert instead of conditional.

Related thread: brave/brave-core#18146 (comment)

[ajayseeker]

Hey @sangwoo108, Can you please assign it to me? I'd like to take it up.

[ajayseeker]

Hi @sangwoo108, wanted to confirm if my understanding is correct,
toolkit_views if a preference which guard multiple features, presently we are adding the following dependency conditionally based on toolkit_views preference is enabled or not.

if (toolkit_views) {
    deps += [ "//ui/views" ]
  }

we want to move away from this statement to asserting that toolkit_views is present assert(toolkit_views). This is because assert statement isn't available in case of android but chrome/browser/extensions is anyways not applicable in android. So asserting won't do any harm. Is this correct?

[ajayseeker]

Also, this file src/chrome/browser/extensions/BUILD.gn isn't in src/brave. Should I create a PR in chromium repo? Or Is there something I'm missing?

[sangwoo108]

toolkit_views is not a preference. toolkit_views is a variable in gn, which set to true only on desktop.

---

This is because assert statement isn't available in case of android but chrome/browser/extensions is anyways not applicable in android. So asserting won't do any harm. Is this correct?

Yes, we'd like to check if //brave/browser/extensions is okay with assert(toolkit_view` instead of if (tookit_views).

---

Also, this file src/chrome/browser/extensions/BUILD.gn isn't in src/brave. Should I create a PR in chromium repo? Or Is there something I'm missing?

Here, //brave/browser/extensions means extensions/BUILD.gn in our repo.

[ajayseeker]

toolkit_views is not a preference. toolkit_views is a variable in gn, which set to true only on desktop.

This is because assert statement isn't available in case of android but chrome/browser/extensions is anyways not applicable in android. So asserting won't do any harm. Is this correct?

Yes, we'd like to check if //brave/browser/extensions is okay with assert(toolkit_view` instead of if (tookit_views).

Also, this file src/chrome/browser/extensions/BUILD.gn isn't in src/brave. Should I create a PR in chromium repo? Or Is there something I'm missing?

Here, //brave/browser/extensions means extensions/BUILD.gn in our repo.

oh! my bad! I should have thought of that.
We only need to run a build and launch Brave for verifying, right?

[sangwoo108]

Yup, that's correct. Other platforms will be verified by CI.

[ajayseeker]

Hey! one lame question, I've been facing this problem for sometime. After I set up brave by cloning, create a new branch for an issue and run the build, post the build I see that build has automatically changed few files which show up under changes in source control of VS Code.
Post working on the issue when I stash all the changes auto created by build and go back to the master branch, take a pull and create another branch for a new issue, I see my build starts failing. It complains of few files missing in src/components. This time I fetched the missing files from origin. Now the build fails with following:

/Users/ajaysingh/Repos/Brave/brave-browser/src
> autoninja -C /Users/ajaysingh/Repos/Brave/brave-browser/src/out/redirect_cc brave/tools/redirect_cc -k 1 --offline
ninja: Entering directory `/Users/ajaysingh/Repos/Brave/brave-browser/src/out/redirect_cc'
[0/1] Regenerating ninja files
WARNING at //brave/tools/redirect_cc/args.gni:7:24: Build argument has no effect.
is_redirect_cc_build = false
                       ^----
The variable "is_redirect_cc_build" was set as a build argument
but never appeared in a declare_args() block in any buildfile.

To view all possible args, run "gn args --list <out_dir>"

The build continued as if that argument was unspecified.

ninja: error: unknown target 'brave/tools/redirect_cc'

Is there any BKM to follow while creating new branches? or Is there any known standard method?

[sangwoo108]

If your old branch is rather stale, you might need to run npm run sync again

[ajayseeker]

@sangwoo108, raise a pr