Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
- Save browser context in BraveRequestInfo.
- Remove GetBrowserContextToUse override in IpfsServiceFactory.
- Remove unused headers.
  • Loading branch information
yrliou committed Dec 3, 2020
1 parent f5fd1a5 commit d9d3733
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 34 deletions.
12 changes: 0 additions & 12 deletions browser/ipfs/ipfs_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,14 @@

#include "brave/browser/ipfs/ipfs_service_factory.h"

#include "base/feature_list.h"
#include "base/path_service.h"
#include "brave/browser/brave_browser_process_impl.h"
#include "brave/browser/profiles/profile_util.h"
#include "brave/components/ipfs/features.h"
#include "brave/components/ipfs/ipfs_constants.h"
#include "brave/components/ipfs/ipfs_service.h"
#include "brave/components/ipfs/ipfs_utils.h"
#include "brave/components/ipfs/pref_names.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/common/channel_info.h"
#include "chrome/common/chrome_paths.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/prefs/pref_service.h"
#include "components/user_prefs/user_prefs.h"
#include "extensions/browser/extension_registry_factory.h"
#include "extensions/browser/extension_system.h"
#include "extensions/browser/extension_system_provider.h"
Expand Down Expand Up @@ -64,9 +57,4 @@ KeyedService* IpfsServiceFactory::BuildServiceInstanceFor(
user_data_dir, chrome::GetChannel());
}

content::BrowserContext* IpfsServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
return chrome::GetBrowserContextRedirectedInIncognito(context);
}

} // namespace ipfs
2 changes: 0 additions & 2 deletions browser/ipfs/ipfs_service_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ class IpfsServiceFactory : public BrowserContextKeyedServiceFactory {
// BrowserContextKeyedServiceFactory overrides:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
content::BrowserContext* GetBrowserContextToUse(
content::BrowserContext* context) const override;

DISALLOW_COPY_AND_ASSIGN(IpfsServiceFactory);
};
Expand Down
4 changes: 3 additions & 1 deletion browser/net/ipfs_redirect_network_delegate_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ namespace ipfs {
int OnBeforeURLRequest_IPFSRedirectWork(
const brave::ResponseCallback& next_callback,
std::shared_ptr<brave::BraveRequestInfo> ctx) {
if (!ctx->resolve_ipfs_enabled)
if (!ctx->browser_context ||
IsIpfsResolveMethodDisabled(ctx->browser_context)) {
return net::OK;
}

GURL new_url;
if (ipfs::TranslateIPFSURI(ctx->request_url, &new_url,
Expand Down
59 changes: 49 additions & 10 deletions browser/net/ipfs_redirect_network_delegate_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,24 @@

#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "base/test/scoped_feature_list.h"
#include "brave/browser/net/url_context.h"
#include "brave/common/translate_network_constants.h"
#include "brave/components/ipfs/features.h"
#include "brave/components/ipfs/ipfs_constants.h"
#include "brave/components/ipfs/ipfs_gateway.h"
#include "brave/components/ipfs/ipfs_utils.h"
#include "brave/components/ipfs/pref_names.h"
#include "chrome/common/channel_info.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/test/browser_task_environment.h"
#include "content/public/test/test_browser_context.h"
#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
#include "net/url_request/url_request_test_util.h"
#include "url/gurl.h"
Expand All @@ -32,19 +42,48 @@ GURL GetPublicGateway() {
const char initiator_cid[] =
"bafybeiemxf5abjwjbikoz4mc3a3dla6ual3jsgpdr4cjr3oz3evfyavhwq";

TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIHTTPScheme) {
} // namespace

namespace ipfs {

class IPFSRedirectNetworkDelegateHelperTest : public testing::Test {
public:
IPFSRedirectNetworkDelegateHelperTest()
: browser_context_(new content::TestBrowserContext()) {}
~IPFSRedirectNetworkDelegateHelperTest() override = default;

void SetUp() override {
feature_list_.InitAndEnableFeature(ipfs::features::kIpfsFeature);
prefs_.registry()->RegisterIntegerPref(
kIPFSResolveMethod, static_cast<int>(IPFSResolveMethodTypes::IPFS_ASK));
user_prefs::UserPrefs::Set(browser_context_.get(), &prefs_);
}

content::TestBrowserContext* browser_context() {
return browser_context_.get();
}

private:
content::BrowserTaskEnvironment task_environment_;
std::unique_ptr<content::TestBrowserContext> browser_context_;
TestingPrefServiceSimple prefs_;
base::test::ScopedFeatureList feature_list_;
};

TEST_F(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIHTTPScheme) {
GURL url("http://a.com/ipfs/QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG");
auto brave_request_info = std::make_shared<brave::BraveRequestInfo>(url);
brave_request_info->browser_context = browser_context();
int rc = ipfs::OnBeforeURLRequest_IPFSRedirectWork(brave::ResponseCallback(),
brave_request_info);
EXPECT_EQ(rc, net::OK);
EXPECT_TRUE(brave_request_info->new_url_spec.empty());
}

TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPFSSchemeLocal) {
TEST_F(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPFSSchemeLocal) {
GURL url("ipfs://QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG");
auto brave_request_info = std::make_shared<brave::BraveRequestInfo>(url);
brave_request_info->resolve_ipfs_enabled = true;
brave_request_info->browser_context = browser_context();
brave_request_info->ipfs_gateway_url = GetLocalGateway();
brave_request_info->initiator_url = ipfs::GetIPFSGatewayURL(
initiator_cid, "",
Expand All @@ -57,10 +96,10 @@ TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPFSSchemeLocal) {
"QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG");
}

TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPFSScheme) {
TEST_F(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPFSScheme) {
GURL url("ipfs://QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG");
auto brave_request_info = std::make_shared<brave::BraveRequestInfo>(url);
brave_request_info->resolve_ipfs_enabled = true;
brave_request_info->browser_context = browser_context();
brave_request_info->ipfs_gateway_url = GetPublicGateway();
brave_request_info->initiator_url =
ipfs::GetIPFSGatewayURL(initiator_cid, "", ipfs::GetDefaultIPFSGateway());
Expand All @@ -72,10 +111,10 @@ TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPFSScheme) {
"https://dweb.link/ipfs/QmfM2r8seH2GiRaC4esTjeraXEachRt8ZsSeGaWTPLyMoG");
}

TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPNSSchemeLocal) {
TEST_F(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPNSSchemeLocal) {
GURL url("ipns://QmSrPmbaUKA3ZodhzPWZnpFgcPMFWF4QsxXbkWfEptTBJd");
auto brave_request_info = std::make_shared<brave::BraveRequestInfo>(url);
brave_request_info->resolve_ipfs_enabled = true;
brave_request_info->browser_context = browser_context();
brave_request_info->ipfs_gateway_url = GetLocalGateway();
brave_request_info->initiator_url = ipfs::GetIPFSGatewayURL(
initiator_cid, "",
Expand All @@ -88,10 +127,10 @@ TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPNSSchemeLocal) {
"QmSrPmbaUKA3ZodhzPWZnpFgcPMFWF4QsxXbkWfEptTBJd");
}

TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPNSScheme) {
TEST_F(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPNSScheme) {
GURL url("ipns://QmSrPmbaUKA3ZodhzPWZnpFgcPMFWF4QsxXbkWfEptTBJd");
auto brave_request_info = std::make_shared<brave::BraveRequestInfo>(url);
brave_request_info->resolve_ipfs_enabled = true;
brave_request_info->browser_context = browser_context();
brave_request_info->ipfs_gateway_url = GetPublicGateway();
brave_request_info->initiator_url =
ipfs::GetIPFSGatewayURL(initiator_cid, "", ipfs::GetDefaultIPFSGateway());
Expand All @@ -103,4 +142,4 @@ TEST(IPFSRedirectNetworkDelegateHelperTest, TranslateIPFSURIIPNSScheme) {
"https://dweb.link/ipns/QmSrPmbaUKA3ZodhzPWZnpFgcPMFWF4QsxXbkWfEptTBJd");
}

} // namespace
} // namespace ipfs
4 changes: 1 addition & 3 deletions browser/net/url_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "brave/components/ipfs/pref_names.h"
#include "brave/components/ipfs/ipfs_constants.h"
#include "brave/components/ipfs/ipfs_gateway.h"
#include "brave/components/ipfs/ipfs_utils.h"
#include "chrome/common/channel_info.h"
#include "components/prefs/testing_pref_service.h"
#include "components/user_prefs/user_prefs.h"
Expand Down Expand Up @@ -138,9 +137,8 @@ std::shared_ptr<brave::BraveRequestInfo> BraveRequestInfo::MakeCTX(
ctx->redirect_source);
ctx->upload_data = GetUploadData(request);

ctx->browser_context = browser_context;
#if BUILDFLAG(IPFS_ENABLED)
ctx->resolve_ipfs_enabled =
!ipfs::IsIpfsResolveMethodDisabled(browser_context);
auto* prefs = user_prefs::UserPrefs::Get(browser_context);
bool local = static_cast<ipfs::IPFSResolveMethodTypes>(
prefs->GetInteger(kIPFSResolveMethod)) ==
Expand Down
2 changes: 1 addition & 1 deletion browser/net/url_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ struct BraveRequestInfo {
bool allow_http_upgradable_resource = false;
bool allow_referrers = false;
bool is_webtorrent_disabled = false;
bool resolve_ipfs_enabled = false;
int render_process_id = 0;
int render_frame_id = 0;
int frame_tree_node_id = 0;
uint64_t request_identifier = 0;
size_t next_url_request_index = 0;

content::BrowserContext* browser_context = nullptr;
net::HttpRequestHeaders* headers = nullptr;
// The following two sets are populated by |OnBeforeStartTransactionCallback|.
// |set_headers| contains headers which values were added or modified.
Expand Down
2 changes: 1 addition & 1 deletion components/ipfs/ipfs_navigation_throttle_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ TEST_F(IpfsNavigationThrottleUnitTest, DeferUntilIpfsProcessLaunched) {
"QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"};

auto* service = ipfs_service(profile());
ASSERT_TRUE(service != nullptr);
ASSERT_TRUE(service);
service->SetSkipGetConnectedPeersCallbackForTest(true);

content::MockNavigationHandle test_handle(web_contents());
Expand Down
9 changes: 5 additions & 4 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,10 @@ test("brave_unit_tests") {
"//components/bookmarks/browser",
"//components/bookmarks/common",
"//components/content_settings/core/test:test_support",
"//components/country_codes",
"//components/domain_reliability:domain_reliability",
"//components/language/core/browser",
"//components/os_crypt:test_support",
"//components/permissions",
"//components/prefs",
"//components/prefs:test_support",
Expand All @@ -179,10 +183,7 @@ test("brave_unit_tests") {
"//components/signin/public/base",
"//components/sync_preferences",
"//components/translate/core/browser:test_support",
"//components/country_codes",
"//components/domain_reliability:domain_reliability",
"//components/language/core/browser",
"//components/os_crypt:test_support",
"//components/user_prefs",
"//components/variations/service:buildflags",
"//components/version_info",
"//content/public/common",
Expand Down

0 comments on commit d9d3733

Please sign in to comment.