Address review feedbacks from iefremov

brave · Oct 9, 2020 · 652e654 · 652e654
1 parent 8007d60
commit 652e654
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 32 deletions.
diff --git a/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html b/browser/resources/settings/brave_default_extensions_page/brave_default_extensions_page.html
@@ -137,11 +137,11 @@
         on-settings-boolean-control-change="onTorEnabledChange_">
     </settings-toggle-button>
     <settings-toggle-button
-      pref="{{prefs.tor.auto_onion_location}}"
-      class="cr-row"
-      label="$i18n{autoOnionLocationLabel}"
-      sub-label="$i18n{autoOnionLocationDesc}"
-      disabled="[[!torEnabled_]]">
+        pref="{{prefs.tor.auto_onion_location}}"
+        class="cr-row"
+        label="$i18n{autoOnionLocationLabel}"
+        sub-label="$i18n{autoOnionLocationDesc}"
+        disabled="[[!torEnabled_]]">
     </settings-toggle-button>
     </if>
     <settings-toggle-button id="webTorrentEnabled"

diff --git a/browser/tor/BUILD.gn b/browser/tor/BUILD.gn
@@ -24,28 +24,26 @@ source_set("tor") {
     "//mojo/public/cpp/bindings",
   ]
 
-  public_deps = [
-    ":buildflags",
-  ]
+  public_deps = [ ":buildflags" ]
 
   if (enable_tor) {
     sources += [
+      "onion_location_navigation_throttle.cc",
+      "onion_location_navigation_throttle.h",
+      "onion_location_tab_helper.cc",
+      "onion_location_tab_helper.h",
       "tor_control.cc",
       "tor_control.h",
       "tor_launcher_factory.cc",
       "tor_launcher_factory.h",
       "tor_navigation_throttle.cc",
       "tor_navigation_throttle.h",
-      "onion_location_navigation_throttle.cc",
-      "onion_location_navigation_throttle.h",
       "tor_profile_service.cc",
       "tor_profile_service.h",
       "tor_profile_service_impl.cc",
       "tor_profile_service_impl.h",
       "tor_tab_helper.cc",
       "tor_tab_helper.h",
-      "onion_location_tab_helper.cc",
-      "onion_location_tab_helper.h",
     ]
 
     deps += [

diff --git a/browser/tor/onion_location_navigation_throttle.cc b/browser/tor/onion_location_navigation_throttle.cc
@@ -26,6 +26,8 @@ namespace {
 
 bool GetOnionLocation(const net::HttpResponseHeaders* headers,
                       std::string* onion_location) {
+  DCHECK(onion_location);
+
   onion_location->clear();
   std::string name = "onion-location";
 
@@ -54,7 +56,8 @@ void OnTorProfileCreated(GURL onion_location,
 std::unique_ptr<OnionLocationNavigationThrottle>
 OnionLocationNavigationThrottle::MaybeCreateThrottleFor(
     content::NavigationHandle* navigation_handle) {
-  if (tor::TorProfileService::IsTorDisabled())
+  if (tor::TorProfileService::IsTorDisabled() ||
+      !navigation_handle->IsInMainFrame())
     return nullptr;
   return std::make_unique<OnionLocationNavigationThrottle>(navigation_handle);
 }
@@ -70,23 +73,24 @@ OnionLocationNavigationThrottle::~OnionLocationNavigationThrottle() {}
 
 content::NavigationThrottle::ThrottleCheckResult
 OnionLocationNavigationThrottle::WillProcessResponse() {
-  if (navigation_handle()->IsInMainFrame()) {
-    auto* headers = navigation_handle()->GetResponseHeaders();
-    std::string onion_location;
-    if (headers && GetOnionLocation(headers, &onion_location) &&
-        !navigation_handle()->GetURL().DomainIs("onion")) {
-      // If user prefers opening it automatically
-      if (profile_->GetPrefs()->GetBoolean(prefs::kAutoOnionLocation)) {
-        profiles::SwitchToTorProfile(
-            base::BindRepeating(&OnTorProfileCreated, GURL(onion_location)));
-      } else {
-        OnionLocationTabHelper::SetOnionLocation(
-            navigation_handle()->GetWebContents(), GURL(onion_location));
-      }
-    } else {
+  auto* headers = navigation_handle()->GetResponseHeaders();
+  std::string onion_location;
+  if (headers && GetOnionLocation(headers, &onion_location) &&
+      !navigation_handle()->GetURL().DomainIs("onion")) {
+    // If user prefers opening it automatically
+    if (profile_->GetPrefs()->GetBoolean(prefs::kAutoOnionLocation)) {
+      // Cleanup previous label before user switching to kAutoOnionLocation
       OnionLocationTabHelper::SetOnionLocation(
           navigation_handle()->GetWebContents(), GURL());
+      profiles::SwitchToTorProfile(
+          base::BindRepeating(&OnTorProfileCreated, GURL(onion_location)));
+    } else {
+      OnionLocationTabHelper::SetOnionLocation(
+          navigation_handle()->GetWebContents(), GURL(onion_location));
     }
+  } else {
+    OnionLocationTabHelper::SetOnionLocation(
+        navigation_handle()->GetWebContents(), GURL());
   }
   return content::NavigationThrottle::PROCEED;
 }
@@ -95,8 +99,7 @@ content::NavigationThrottle::ThrottleCheckResult
 OnionLocationNavigationThrottle::WillStartRequest() {
   if (!brave::IsTorProfile(profile_)) {
     GURL url = navigation_handle()->GetURL();
-    if (url.SchemeIsHTTPOrHTTPS() && url.DomainIs("onion") &&
-        navigation_handle()->IsInMainFrame()) {
+    if (url.SchemeIsHTTPOrHTTPS() && url.DomainIs("onion")) {
       profiles::SwitchToTorProfile(
           base::BindRepeating(&OnTorProfileCreated, std::move(url)));
       return content::NavigationThrottle::CANCEL_AND_IGNORE;

diff --git a/browser/ui/views/brave_actions/brave_actions_container.cc b/browser/ui/views/brave_actions/brave_actions_container.cc
@@ -39,7 +39,6 @@
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/view.h"
 
-
 class BraveActionsContainer::EmptyExtensionsContainer
     : public ExtensionsContainer {
  public:

diff --git a/browser/ui/views/brave_actions/brave_actions_container.h b/browser/ui/views/brave_actions/brave_actions_container.h
@@ -27,8 +27,8 @@
 #endif
 
 class BraveActionViewController;
-class OnionLocationView;
 class BraveActionsContainerTest;
+class OnionLocationView;
 class RewardsBrowserTest;
 
 namespace extensions {

diff --git a/browser/ui/views/location_bar/onion_location_view.h b/browser/ui/views/location_bar/onion_location_view.h
@@ -6,7 +6,6 @@
 #ifndef BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_ONION_LOCATION_VIEW_H_
 #define BRAVE_BROWSER_UI_VIEWS_LOCATION_BAR_ONION_LOCATION_VIEW_H_
 
-#include "chrome/browser/profiles/profile.h"
 #include "ui/views/controls/button/label_button.h"
 
 class Profile;