Skip to content

Commit

Permalink
Refactor the implementation of the webNavigation extension API.
Browse files Browse the repository at this point in the history 
The webNavigation extension API does not work properly with out-of-process
iframes and will be broken with the upcoming changes in navigation
code (PlzNavigate). This CL is aiming to refactor the implementation to
use the NavigationHandle API, which is compatible with PlzNavigate and
helps hide some of the nasty details of cross-process navigations.

This CL depends on new APIs added to NavigationHandle in https://codereview.chromium.org/1667163002/.

BUG=584493
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review URL: https://codereview.chromium.org/1670673003

Cr-Commit-Position: refs/heads/master@{#374505}
  • Loading branch information
@naskooskov
naskooskov authored and Commit bot committed Feb 9, 2016
1 parent fbcb5de commit e419e21
Show file tree
Hide file tree
Showing 60 changed files with 409 additions and 519 deletions.
60 changes: 12 additions & 48 deletions chrome/browser/extensions/api/web_navigation/frame_navigation_state.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,21 @@ bool FrameNavigationState::CanSendEvents(
return IsValidUrl(it->second.url);
}

void FrameNavigationState::StartTrackingNavigation(
void FrameNavigationState::StartTrackingDocumentLoad(
content::RenderFrameHost* frame_host,
const GURL& url,
bool is_same_page,
bool is_error_page,
bool is_iframe_srcdoc) {
FrameState& frame_state = frame_host_state_map_[frame_host];
frame_state.error_occurred = is_error_page;
frame_state.url = url;
frame_state.is_iframe_srcdoc = is_iframe_srcdoc;
DCHECK(!is_iframe_srcdoc || url == GURL(url::kAboutBlankURL));
frame_state.is_navigating = true;
frame_state.is_committed = false;
frame_state.is_server_redirected = false;
frame_state.is_parsing = true;
if (!is_same_page) {
frame_state.is_loading = true;
frame_state.is_parsing = true;
}
}

void FrameNavigationState::FrameHostCreated(
Expand Down Expand Up @@ -107,10 +108,9 @@ bool FrameNavigationState::IsValidFrame(
GURL FrameNavigationState::GetUrl(content::RenderFrameHost* frame_host) const {
FrameHostToStateMap::const_iterator it =
frame_host_state_map_.find(frame_host);
if (it == frame_host_state_map_.end()) {
NOTREACHED();
if (it == frame_host_state_map_.end())
return GURL();
}

if (it->second.is_iframe_srcdoc)
return GURL(content::kAboutSrcDocURL);
return it->second.url;
Expand All @@ -134,22 +134,22 @@ bool FrameNavigationState::GetErrorOccurredInFrame(
return it == frame_host_state_map_.end() || it->second.error_occurred;
}

void FrameNavigationState::SetNavigationCompleted(
void FrameNavigationState::SetDocumentLoadCompleted(
content::RenderFrameHost* frame_host) {
FrameHostToStateMap::iterator it = frame_host_state_map_.find(frame_host);
if (it == frame_host_state_map_.end()) {
NOTREACHED();
return;
}
it->second.is_navigating = false;
it->second.is_loading = false;
}

bool FrameNavigationState::GetNavigationCompleted(
bool FrameNavigationState::GetDocumentLoadCompleted(
content::RenderFrameHost* frame_host) const {
FrameHostToStateMap::const_iterator it =
frame_host_state_map_.find(frame_host);
DCHECK(it != frame_host_state_map_.end());
return it == frame_host_state_map_.end() || !it->second.is_navigating;
return it == frame_host_state_map_.end() || !it->second.is_loading;
}

void FrameNavigationState::SetParsingFinished(
Expand All @@ -170,40 +170,4 @@ bool FrameNavigationState::GetParsingFinished(
return it == frame_host_state_map_.end() || !it->second.is_parsing;
}

void FrameNavigationState::SetNavigationCommitted(
content::RenderFrameHost* frame_host) {
FrameHostToStateMap::iterator it = frame_host_state_map_.find(frame_host);
if (it == frame_host_state_map_.end()) {
NOTREACHED();
return;
}
it->second.is_committed = true;
}

bool FrameNavigationState::GetNavigationCommitted(
content::RenderFrameHost* frame_host) const {
FrameHostToStateMap::const_iterator it =
frame_host_state_map_.find(frame_host);
DCHECK(it != frame_host_state_map_.end());
return it != frame_host_state_map_.end() && it->second.is_committed;
}

void FrameNavigationState::SetIsServerRedirected(
content::RenderFrameHost* frame_host) {
FrameHostToStateMap::iterator it = frame_host_state_map_.find(frame_host);
if (it == frame_host_state_map_.end()) {
NOTREACHED();
return;
}
it->second.is_server_redirected = true;
}

bool FrameNavigationState::GetIsServerRedirected(
content::RenderFrameHost* frame_host) const {
FrameHostToStateMap::const_iterator it =
frame_host_state_map_.find(frame_host);
DCHECK(it != frame_host_state_map_.end());
return it != frame_host_state_map_.end() && it->second.is_server_redirected;
}

} // namespace extensions
38 changes: 12 additions & 26 deletions chrome/browser/extensions/api/web_navigation/frame_navigation_state.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class RenderViewHost;

namespace extensions {

// Tracks the navigation state of all frame hosts in a given tab currently known
// Tracks the loading state of all frame hosts in a given tab currently known
// to the webNavigation API. It is mainly used to track in which frames an error
// occurred so no further events for this frame are being sent.
class FrameNavigationState {
Expand All @@ -39,11 +39,12 @@ class FrameNavigationState {
// True if navigation events for the given frame can be sent.
bool CanSendEvents(content::RenderFrameHost* frame_host) const;

// Starts to track a navigation in |frame_host| to |url|.
void StartTrackingNavigation(content::RenderFrameHost* frame_host,
const GURL& url,
bool is_error_page,
bool is_iframe_srcdoc);
// Starts to track a document load in |frame_host| to |url|.
void StartTrackingDocumentLoad(content::RenderFrameHost* frame_host,
const GURL& url,
bool is_same_page,
bool is_error_page,
bool is_iframe_srcdoc);

// Adds the |frame_host| to the set of RenderFrameHosts associated with the
// WebContents this object is tracking. This method and FrameHostDeleted
Expand Down Expand Up @@ -73,32 +74,19 @@ class FrameNavigationState {
// True if |frame_host| is marked as being in an error state.
bool GetErrorOccurredInFrame(content::RenderFrameHost* frame_host) const;

// Marks |frame_host| as having finished its last navigation, i.e. the
// Marks |frame_host| as having finished its last document load, i.e. the
// onCompleted event was fired for this frame.
void SetNavigationCompleted(content::RenderFrameHost* frame_host);
void SetDocumentLoadCompleted(content::RenderFrameHost* frame_host);

// True if |frame_host| is currently not navigating.
bool GetNavigationCompleted(content::RenderFrameHost* frame_host) const;
// True if |frame_host| is currently not loading a document.
bool GetDocumentLoadCompleted(content::RenderFrameHost* frame_host) const;

// Marks |frame_host| as having finished parsing.
void SetParsingFinished(content::RenderFrameHost* frame_host);

// True if |frame_host| has finished parsing.
bool GetParsingFinished(content::RenderFrameHost* frame_host) const;

// Marks |frame_host| as having committed its navigation, i.e. the onCommitted
// event was fired for this frame.
void SetNavigationCommitted(content::RenderFrameHost* frame_host);

// True if |frame_host| has committed its navigation.
bool GetNavigationCommitted(content::RenderFrameHost* frame_host) const;

// Marks |frame_host| as redirected by the server.
void SetIsServerRedirected(content::RenderFrameHost* frame_host);

// True if |frame_host| was redirected by the server.
bool GetIsServerRedirected(content::RenderFrameHost* frame_host) const;

#ifdef UNIT_TEST
static void set_allow_extension_scheme(bool allow_extension_scheme) {
allow_extension_scheme_ = allow_extension_scheme;
Expand All @@ -111,9 +99,7 @@ class FrameNavigationState {

bool error_occurred; // True if an error has occurred in this frame.
bool is_iframe_srcdoc; // True if the frame is displaying its srcdoc.
bool is_navigating; // True if there is a navigation going on.
bool is_committed; // True if the navigation is already committed.
bool is_server_redirected; // True if a server redirect happened.
bool is_loading; // True if there is a document load going on.
bool is_parsing; // True if the frame is still parsing.
GURL url; // URL of this frame.
};
Expand Down
46 changes: 28 additions & 18 deletions chrome/browser/extensions/api/web_navigation/frame_navigation_state_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ TEST_F(FrameNavigationStateTest, TrackFrame) {
// Create a main frame.
EXPECT_FALSE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_FALSE(navigation_state_.IsValidFrame(main_rfh()));
navigation_state_.StartTrackingNavigation(main_rfh(), url1, false, false);
navigation_state_.SetNavigationCommitted(main_rfh());
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url1, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_TRUE(navigation_state_.IsValidFrame(main_rfh()));

Expand All @@ -49,8 +49,8 @@ TEST_F(FrameNavigationStateTest, TrackFrame) {
content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
EXPECT_FALSE(navigation_state_.CanSendEvents(sub_frame));
EXPECT_FALSE(navigation_state_.IsValidFrame(sub_frame));
navigation_state_.StartTrackingNavigation(sub_frame, url2, false, false);
navigation_state_.SetNavigationCommitted(sub_frame);
navigation_state_.StartTrackingDocumentLoad(sub_frame, url2, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(sub_frame));
EXPECT_TRUE(navigation_state_.IsValidFrame(sub_frame));

Expand All @@ -73,7 +73,8 @@ TEST_F(FrameNavigationStateTest, TrackFrame) {
TEST_F(FrameNavigationStateTest, ErrorState) {
const GURL url("http://www.google.com/");

navigation_state_.StartTrackingNavigation(main_rfh(), url, false, false);
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_FALSE(navigation_state_.GetErrorOccurredInFrame(main_rfh()));

Expand All @@ -83,12 +84,14 @@ TEST_F(FrameNavigationStateTest, ErrorState) {
EXPECT_TRUE(navigation_state_.GetErrorOccurredInFrame(main_rfh()));

// Navigations to a network error page should be ignored.
navigation_state_.StartTrackingNavigation(main_rfh(), GURL(), true, false);
navigation_state_.StartTrackingDocumentLoad(main_rfh(), GURL(), false, true,
false);
EXPECT_FALSE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_TRUE(navigation_state_.GetErrorOccurredInFrame(main_rfh()));

// However, when the frame navigates again, it should send events again.
navigation_state_.StartTrackingNavigation(main_rfh(), url, false, false);
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_FALSE(navigation_state_.GetErrorOccurredInFrame(main_rfh()));
}
Expand All @@ -100,8 +103,10 @@ TEST_F(FrameNavigationStateTest, ErrorStateFrame) {

content::RenderFrameHost* sub_frame =
content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
navigation_state_.StartTrackingNavigation(main_rfh(), url, false, false);
navigation_state_.StartTrackingNavigation(sub_frame, url, false, false);
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url, false, false,
false);
navigation_state_.StartTrackingDocumentLoad(sub_frame, url, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_TRUE(navigation_state_.CanSendEvents(sub_frame));

Expand All @@ -111,12 +116,14 @@ TEST_F(FrameNavigationStateTest, ErrorStateFrame) {
EXPECT_FALSE(navigation_state_.CanSendEvents(sub_frame));

// Navigations to a network error page should be ignored.
navigation_state_.StartTrackingNavigation(sub_frame, GURL(), true, false);
navigation_state_.StartTrackingDocumentLoad(sub_frame, GURL(), false, true,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_FALSE(navigation_state_.CanSendEvents(sub_frame));

// However, when the frame navigates again, it should send events again.
navigation_state_.StartTrackingNavigation(sub_frame, url, false, false);
navigation_state_.StartTrackingDocumentLoad(sub_frame, url, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_TRUE(navigation_state_.CanSendEvents(sub_frame));
}
Expand All @@ -125,7 +132,8 @@ TEST_F(FrameNavigationStateTest, ErrorStateFrame) {
TEST_F(FrameNavigationStateTest, WebSafeScheme) {
const GURL url("unsafe://www.google.com/");

navigation_state_.StartTrackingNavigation(main_rfh(), url, false, false);
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url, false, false,
false);
EXPECT_FALSE(navigation_state_.CanSendEvents(main_rfh()));
}

Expand All @@ -137,8 +145,10 @@ TEST_F(FrameNavigationStateTest, SrcDoc) {

content::RenderFrameHost* sub_frame =
content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
navigation_state_.StartTrackingNavigation(main_rfh(), url, false, false);
navigation_state_.StartTrackingNavigation(sub_frame, blank, false, true);
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url, false, false,
false);
navigation_state_.StartTrackingDocumentLoad(sub_frame, blank, false, false,
true);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_TRUE(navigation_state_.CanSendEvents(sub_frame));

Expand All @@ -156,8 +166,8 @@ TEST_F(FrameNavigationStateTest, DetachFrame) {
// Create a main frame.
EXPECT_FALSE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_FALSE(navigation_state_.IsValidFrame(main_rfh()));
navigation_state_.StartTrackingNavigation(main_rfh(), url1, false, false);
navigation_state_.SetNavigationCommitted(main_rfh());
navigation_state_.StartTrackingDocumentLoad(main_rfh(), url1, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(main_rfh()));
EXPECT_TRUE(navigation_state_.IsValidFrame(main_rfh()));

Expand All @@ -166,8 +176,8 @@ TEST_F(FrameNavigationStateTest, DetachFrame) {
content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
EXPECT_FALSE(navigation_state_.CanSendEvents(sub_frame));
EXPECT_FALSE(navigation_state_.IsValidFrame(sub_frame));
navigation_state_.StartTrackingNavigation(sub_frame, url2, false, false);
navigation_state_.SetNavigationCommitted(sub_frame);
navigation_state_.StartTrackingDocumentLoad(sub_frame, url2, false, false,
false);
EXPECT_TRUE(navigation_state_.CanSendEvents(sub_frame));
EXPECT_TRUE(navigation_state_.IsValidFrame(sub_frame));

Expand Down
Loading

0 comments on commit e419e21

Please sign in to comment.