New welcome flow for default search

[imptrx]

• Change flow for how users select their default search engine. Gives user ability to use immediate dropdown rather than go through settings
• See Default search engine functional UI changes brave-ui#472 for storyboard and new SVG change
• fix add default search engine functionality to onboarding process brave-browser#1548

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).
• Verified that these changes build without errors on
  • Windows
  • macOS
  • Linux
• Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) on
  • Windows
  • macOS
  • Linux
• Verified that all lint errors/warnings are resolved (npm run lint)
• Ran git rebase master (if needed).
• 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.
• Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone
• Public documentation has been updated as necessary. For instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protection-Mode
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Custom-Headers
  • https://github.com/brave/brave-browser/wiki/Web-compatibility-issues-with-tracking-protection

Test Plan:

• Ensure dropdown works
• Ensure button is disabled if no search engine is selected
• Ensure display text changes based on locale
• (default) should be appended to the region's default search provider
• Ensure 'Confirm' button still links to next page of on-boarding flow
• Ensure responsiveness on smaller screens
• Ensure new search engine selected becomes the default search engine in address bad

Reviewer Checklist:

• New files have MPL-2.0 license header.
• Request a security/privacy review as needed.
• Adequate test coverage exists to prevent regressions
• Verify test plan is specified in PR before merging to source

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on.
• All relevant documentation has been updated.

[cezaraugusto]

left comments regarding locale

[cezaraugusto]

thanks @imptrx! front-end is good to go

[bsclifton]

Will start checking out the back end parts today 😄

[bsclifton]

Example entry (returned from the SearchEnginesHandler):

{
  "canBeDefault": true,
  "canBeEdited": true,
  "canBeRemoved": true,
  "default": false,
  "displayName": "DuckDuckGo",
  "iconURL": "https://duckduckgo.com/favicon.ico",
  "id": 3,
  "isOmniboxExtension": false,
  "keyword": ":d",
  "modelIndex": 1,
  "name": "DuckDuckGo",
  "url": "https://duckduckgo.com/?q=%s&t=brave",
  "urlLocked": true
}

[cezaraugusto]

@imptrx could you move this interface to welcome.d.ts file?

[bsclifton]

@rebron @imptrx @cezaraugusto let's meet up on this one 😄

Because of how I implemented the back-end (it sets it immediately), the button next to the drop down likely isn't needed (at the moment, it does the same thing as hitting next, but that's all it does)

[cezaraugusto]

@rossmoody thoughts on #2479 (comment)?

[rossmoody]

I built this branch and I actually quite like the functionality we have going on here. I think it's important we keep the button, though. Both cause it signals to the user to make a decision by being disabled but also because it gives them a place to feel like they "confirmed" the choice (even if technically the setting is in place already) instead of advancing what might seem abruptly.

[bsclifton]

Awesome, thanks @rossmoody 😄I guess this is ready for official review! Did you want to be one of the lucky people to hit the review button?

edit: fixing lint...

[imptrx]

@bsclifton there was a more detailed spec that got posted in the sister PR: brave/brave-ui#472 (comment). I can address those details as well as the changes requested

[bsclifton]

@imptrx awesome, sounds great! 😄 Thanks

[imptrx]

@bsclifton @cezaraugusto I think this PR is good for another review 😄I've moved the majority of the chrome API logic into redux and added some tests around those. Once everything checks out I'll clean up/squash the commits into chunks

[bsclifton]

Code changes look great! Nice and clean. Code also works as expected. Great job with squashing on this one. 😻

Don't forget to add closing keywords to the commit! For example:
Fixes https://github.com/brave/brave-browser/issues/1548
(so that GitHub will auto-close the issue when this is merged)

[bsclifton]

Quick note- I did see this in the console - not sure if it's a show-stopper (I compiled release mode)

[52084:775:0611/131046.411818:ERROR:CONSOLE(496)] "Uncaught TypeError: Cannot read property 'name' of undefined", source: chrome://welcome/brave_welcome.bundle.js (496)
[52084:775:0611/131046.430382:ERROR:CONSOLE(38339)] "The above error occurred in the <SearchEngineBox> component:
    in SearchEngineBox (created by WelcomePage)
    in div (created by styled.div)
    in styled.div (created by WelcomePage)
    in div (created by styled.div)
    in styled.div (created by WelcomePage)
    in div (created by styled.div)
    in styled.div (created by WelcomePage)
    in WelcomePage (created by Connect(WelcomePage))
    in Connect(WelcomePage)
    in ThemeProvider
    in Provider

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries.", source: chrome://welcome/brave_welcome.bundle.js (38339)

[imptrx]

@bsclifton @cezaraugusto @rebron It looks like when we initially load the welcome(with a blank profile) here is the current behavior:

1. Brave opens the welcome web-ui
2. The page tries to fetch a list of search providers
3. Page crashes because the window.cr.sendWithPromise('getSearchEnginesList') doesn't return anything
4. Doing some other action in Brave will somehow trigger a searchEngineList to be set
5. If we load the welcome page again, things will work as expected

So I believe the issue as mentioned earlier is that Brave needs to set a search engine list (along with the current partner default provider for the region) before we try to load any pages. Would that be something we can do on the C++ side?

[bsclifton]

@imptrx checking it out now...

[bsclifton]

@imptrx I pushed a fix and verified it works with fresh profiles. I believe a race condition was happening

• request is made
• control is rendered
• error occurred (no .name on undefined)
• (response would have came back here)
• updating props would have triggered a re-render

With this updated logic, it'll render twice. Once before the request finishes and once after 👍

[cezaraugusto]

++ great work here! clean code, manual test pass, unit tests cover key features. nice work @imptrx @bsclifton! I'd approve this 10x

[cezaraugusto]

waiting on Jenkins before merge

[bsclifton]

😎👌