Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix infinite OpenInTorWindow looping when auto redirect through navigation throttle. #7713

Merged
merged 1 commit into from
Jan 27, 2021

Conversation

darkdh
Copy link
Member

@darkdh darkdh commented Jan 27, 2021

Also change window disposition to SWITCH_TO_TAB to avoid redundant new
tab.
We no longer close the original tab to avoid confusion and lots of users complained about this behavior.

The crash cause is calling chrome::FindTabbedBrowser with match_original_profiles equals true so OnionLocationNavigationThrottle::WillStartRequest got triggered endlessly.

Resolves brave/brave-browser#13736

Submitter Checklist:

  • There is a ticket for my issue.
  • Used Github auto-closing keywords in the commit message.
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed).
  • Requested a security/privacy review as needed.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Crash

brave/brave-browser#13736 (comment)

Window disposition and original tab

  1. Clean profile
  2. Turn on Automatically redirect .onion sites in brave://settings/extensions
  3. In normal window open: brave.com
  4. There will be only one tab opened which is ttps://brave5t5rjjg3s6k.onion and no new tab in Tor window
  5. The original tab in normal window won't be closed
  6. In normal window open: https://www.torproject.org/
  7. The second tab http://expyuzz4wqqyqhjn.onion/index.html will be opened in Tor window
  8. Repeat step 3 and step 6
  9. There will still be two tabs, https://brave5t5rjjg3s6k.onion and http://expyuzz4wqqyqhjn.onion/index.html

@darkdh darkdh requested a review from bbondy January 27, 2021 01:13
@darkdh darkdh self-assigned this Jan 27, 2021
navigation throttle.
Also change window disposition to SWITCH_TO_TAB to avoid redundant new
tab. We no longer close the original tab to avoid confusion.
@darkdh
Copy link
Member Author

darkdh commented Jan 27, 2021

@darkdh darkdh merged commit 89e0c12 into master Jan 27, 2021
@darkdh darkdh deleted the auto-tor-redirect-crash branch January 27, 2021 16:59
@darkdh darkdh added this to the 1.21.x - Nightly milestone Jan 27, 2021
darkdh added a commit that referenced this pull request Feb 23, 2021
Fix infinite OpenInTorWindow looping when auto redirect through navigation throttle.
@stephendonner
Copy link
Contributor

stephendonner commented Feb 23, 2021

Verified FIXED using the testplan from #7713 (comment) on

Brave 1.22.43 Chromium: 89.0.4389.58 (Official Build) nightly (x86_64)
Revision 1a139f28ecc27719439e37c6b1533cee999cb802-refs/branch-heads/4389@{#1134}
OS macOS Version 11.2.1 (Build 20D74)
Brave 1.22.43 Chromium: 89.0.4389.58 (Official Build) nightly (64-bit)
Revision 1a139f28ecc27719439e37c6b1533cee999cb802-refs/branch-heads/4389@{#1134}
OS Windows 10 OS Version 2009 (Build 21318.1000)
  1. Clean profile
  2. Enabled Automatically redirect .onion sites in brave://settings/extensions
  3. In normal window opened: brave.com
  4. Verified only one tab opened which is https://brave5t5rjjg3s6k.onion and no new tab in a Tor window

Screen Shot 2021-02-23 at 3 30 31 PM

  1. The original tab in normal window wasn't closed
  2. In a new normal window, I opened: https://www.torproject.org/
  3. The second tab http://expyuzz4wqqyqhjn.onion/index.html was opened in a Tor window

Screen Shot 2021-02-23 at 3 32 27 PM

  1. Repeated step 3 and step 6
  2. Confirmed there were still only two Tor tabs, https://brave5t5rjjg3s6k.onion and http://expyuzz4wqqyqhjn.onion/index.html

Screen Shot 2021-02-23 at 3 26 54 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash on opening .onion link
3 participants