Commit 3f2c87f3 authored by Charles Harrison's avatar Charles Harrison Committed by Commit Bot

TabSpecificContentSettings: remove redundant GURL copies

Don't need to store the previous_url, NavigationHandle does it for us.

This removes 1 copy on navigation start and 4 copies on commit. It
also simplifies the code a reasonable amount.

Bug: None
Change-Id: I1f19f87a254c6b6fb5147a38da23f66b5db55c01
Reviewed-on: https://chromium-review.googlesource.com/774985Reviewed-by: default avatarMartin Šrámek <msramek@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517499}
parent 9bdca957
...@@ -15,13 +15,6 @@ ...@@ -15,13 +15,6 @@
namespace { namespace {
ContentSettingsUsagesState::CommittedDetails CreateDetailsWithURL(
const GURL& url) {
ContentSettingsUsagesState::CommittedDetails details;
details.current_url = url;
return details;
}
class ContentSettingsUsagesStateTests : public testing::Test { class ContentSettingsUsagesStateTests : public testing::Test {
public: public:
ContentSettingsUsagesStateTests() = default; ContentSettingsUsagesStateTests() = default;
...@@ -33,9 +26,7 @@ class ContentSettingsUsagesStateTests : public testing::Test { ...@@ -33,9 +26,7 @@ class ContentSettingsUsagesStateTests : public testing::Test {
HostContentSettingsMapFactory::GetForProfile(&profile), type); HostContentSettingsMapFactory::GetForProfile(&profile), type);
GURL url_0("http://www.example.com"); GURL url_0("http://www.example.com");
ContentSettingsUsagesState::CommittedDetails details = state.DidNavigate(url_0, GURL());
CreateDetailsWithURL(url_0);
state.DidNavigate(details);
HostContentSettingsMapFactory::GetForProfile(&profile) HostContentSettingsMapFactory::GetForProfile(&profile)
->SetContentSettingDefaultScope(url_0, url_0, type, std::string(), ->SetContentSettingDefaultScope(url_0, url_0, type, std::string(),
...@@ -105,15 +96,13 @@ class ContentSettingsUsagesStateTests : public testing::Test { ...@@ -105,15 +96,13 @@ class ContentSettingsUsagesStateTests : public testing::Test {
state.OnPermissionSet(url_0, true); state.OnPermissionSet(url_0, true);
details.previous_url = url_0; state.DidNavigate(url_0, url_0);
state.DidNavigate(details);
ContentSettingsUsagesState::StateMap new_state_map = ContentSettingsUsagesState::StateMap new_state_map =
state.state_map(); state.state_map();
EXPECT_EQ(state_map.size(), new_state_map.size()); EXPECT_EQ(state_map.size(), new_state_map.size());
details.current_url = GURL("http://foo.com"); state.DidNavigate(GURL("http://foo.com/"), url_0);
state.DidNavigate(details);
EXPECT_TRUE(state.state_map().empty()); EXPECT_TRUE(state.state_map().empty());
...@@ -130,9 +119,7 @@ class ContentSettingsUsagesStateTests : public testing::Test { ...@@ -130,9 +119,7 @@ class ContentSettingsUsagesStateTests : public testing::Test {
HostContentSettingsMapFactory::GetForProfile(&profile), type); HostContentSettingsMapFactory::GetForProfile(&profile), type);
GURL url_0("http://www.example.com"); GURL url_0("http://www.example.com");
ContentSettingsUsagesState::CommittedDetails details = state.DidNavigate(url_0, GURL());
CreateDetailsWithURL(url_0);
state.DidNavigate(details);
HostContentSettingsMapFactory::GetForProfile(&profile) HostContentSettingsMapFactory::GetForProfile(&profile)
->SetContentSettingDefaultScope(url_0, url_0, type, std::string(), ->SetContentSettingDefaultScope(url_0, url_0, type, std::string(),
......
...@@ -779,13 +779,6 @@ void TabSpecificContentSettings::DidStartNavigation( ...@@ -779,13 +779,6 @@ void TabSpecificContentSettings::DidStartNavigation(
return; return;
} }
const content::NavigationController& controller =
web_contents()->GetController();
content::NavigationEntry* last_committed_entry =
controller.GetLastCommittedEntry();
if (last_committed_entry)
previous_url_ = last_committed_entry->GetURL();
// If we're displaying a network error page do not reset the content // If we're displaying a network error page do not reset the content
// settings delegate's cookies so the user has a chance to modify cookie // settings delegate's cookies so the user has a chance to modify cookie
// settings. // settings.
...@@ -851,19 +844,14 @@ void TabSpecificContentSettings::ClearMidiContentSettings() { ...@@ -851,19 +844,14 @@ void TabSpecificContentSettings::ClearMidiContentSettings() {
void TabSpecificContentSettings::GeolocationDidNavigate( void TabSpecificContentSettings::GeolocationDidNavigate(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
ContentSettingsUsagesState::CommittedDetails committed_details; geolocation_usages_state_.DidNavigate(navigation_handle->GetURL(),
committed_details.current_url = navigation_handle->GetURL(); navigation_handle->GetPreviousURL());
committed_details.previous_url = previous_url_;
geolocation_usages_state_.DidNavigate(committed_details);
} }
void TabSpecificContentSettings::MidiDidNavigate( void TabSpecificContentSettings::MidiDidNavigate(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
ContentSettingsUsagesState::CommittedDetails committed_details; midi_usages_state_.DidNavigate(navigation_handle->GetURL(),
committed_details.current_url = navigation_handle->GetURL(); navigation_handle->GetPreviousURL());
committed_details.previous_url = previous_url_;
midi_usages_state_.DidNavigate(committed_details);
} }
void TabSpecificContentSettings::BlockAllContentForTesting() { void TabSpecificContentSettings::BlockAllContentForTesting() {
......
...@@ -457,9 +457,6 @@ class TabSpecificContentSettings ...@@ -457,9 +457,6 @@ class TabSpecificContentSettings
std::string media_stream_requested_audio_device_; std::string media_stream_requested_audio_device_;
std::string media_stream_requested_video_device_; std::string media_stream_requested_video_device_;
// Holds the previous committed url during a navigation.
GURL previous_url_;
// Observer to watch for content settings changed. // Observer to watch for content settings changed.
ScopedObserver<HostContentSettingsMap, content_settings::Observer> observer_; ScopedObserver<HostContentSettingsMap, content_settings::Observer> observer_;
......
...@@ -26,11 +26,12 @@ void ContentSettingsUsagesState::OnPermissionSet( ...@@ -26,11 +26,12 @@ void ContentSettingsUsagesState::OnPermissionSet(
allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK; allowed ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
} }
void ContentSettingsUsagesState::DidNavigate(const CommittedDetails& details) { void ContentSettingsUsagesState::DidNavigate(const GURL& url,
embedder_url_ = details.current_url; const GURL& previous_url) {
embedder_url_ = url;
if (state_map_.empty()) if (state_map_.empty())
return; return;
if (details.previous_url.GetOrigin() != details.current_url.GetOrigin()) { if (previous_url.GetOrigin() != url.GetOrigin()) {
state_map_.clear(); state_map_.clear();
return; return;
} }
......
...@@ -20,12 +20,6 @@ class HostContentSettingsMap; ...@@ -20,12 +20,6 @@ class HostContentSettingsMap;
// the content setting usage. // the content setting usage.
class ContentSettingsUsagesState { class ContentSettingsUsagesState {
public: public:
// Information about navigation.
struct CommittedDetails {
GURL current_url;
GURL previous_url;
};
ContentSettingsUsagesState( ContentSettingsUsagesState(
HostContentSettingsMap* host_content_settings_map, HostContentSettingsMap* host_content_settings_map,
ContentSettingsType type); ContentSettingsType type);
...@@ -41,7 +35,7 @@ class ContentSettingsUsagesState { ...@@ -41,7 +35,7 @@ class ContentSettingsUsagesState {
// Delegated by WebContents to indicate a navigation has happened and we // Delegated by WebContents to indicate a navigation has happened and we
// may need to clear our settings. // may need to clear our settings.
void DidNavigate(const CommittedDetails& details); void DidNavigate(const GURL& url, const GURL& previous_url);
void ClearStateMap(); void ClearStateMap();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment