Skip to content

Commit

Permalink
Remove unused IPC variable on History navigation API.
Browse files Browse the repository at this point in the history
Now that back/forward buttons processing is moved to the browser we can
remove the need for the extra IPC variable. All calls now are solely
from javascript.

BUG=705583

Change-Id: Iec303699cda252501c147c6fb9a310f259de6be7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1788230
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695143}
  • Loading branch information
dtapuska authored and Commit Bot committed Sep 10, 2019
1 parent 3872052 commit 043b94a
Show file tree
Hide file tree
Showing 12 changed files with 20 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4077,7 +4077,7 @@ TEST_F(NavigationControllerTest, HistoryNavigate) {
process()->sink().ClearMessages();

// Simulate the page calling history.back(). It should create a pending entry.
contents()->OnGoToEntryAtOffset(main_test_rfh(), -1, false, true);
contents()->OnGoToEntryAtOffset(main_test_rfh(), -1, false);
EXPECT_EQ(0, controller.GetPendingEntryIndex());

// Also make sure we told the page to navigate.
Expand All @@ -4087,7 +4087,7 @@ TEST_F(NavigationControllerTest, HistoryNavigate) {
process()->sink().ClearMessages();

// Now test history.forward()
contents()->OnGoToEntryAtOffset(main_test_rfh(), 2, false, true);
contents()->OnGoToEntryAtOffset(main_test_rfh(), 2, false);
EXPECT_EQ(2, controller.GetPendingEntryIndex());

nav_url = GetLastNavigationURL();
Expand All @@ -4098,8 +4098,8 @@ TEST_F(NavigationControllerTest, HistoryNavigate) {
controller.DiscardNonCommittedEntries();

// Make sure an extravagant history.go() doesn't break.
contents()->OnGoToEntryAtOffset(main_test_rfh(), 120, false,
true); // Out of bounds.
contents()->OnGoToEntryAtOffset(main_test_rfh(), 120,
false); // Out of bounds.
EXPECT_EQ(-1, controller.GetPendingEntryIndex());
EXPECT_FALSE(HasNavigationRequest());
}
Expand Down
9 changes: 2 additions & 7 deletions content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4837,8 +4837,7 @@ void WebContentsImpl::OnDidFinishLoad(RenderFrameHostImpl* source,

void WebContentsImpl::OnGoToEntryAtOffset(RenderFrameHostImpl* source,
int offset,
bool has_user_gesture,
bool from_script) {
bool has_user_gesture) {
// Non-user initiated navigations coming from the renderer should be ignored
// if there is an ongoing browser-initiated navigation.
// See https://crbug.com/879965.
Expand All @@ -4856,11 +4855,7 @@ void WebContentsImpl::OnGoToEntryAtOffset(RenderFrameHostImpl* source,

// All frames are allowed to navigate the global history.
if (!delegate_ || delegate_->OnGoToEntryOffset(offset)) {
// We only check sandboxed navigation permissions on navigations originating
// from scripts, and not mouse back buttons (from_script == false), which
// also use this path.
if (from_script &&
source->IsSandboxed(blink::WebSandboxFlags::kTopNavigation)) {
if (source->IsSandboxed(blink::WebSandboxFlags::kTopNavigation)) {
// Keep track of whether this is a session history from a sandboxed iframe
// with top level navigation disallowed.
controller_.GoToOffsetInSandboxedFrame(offset,
Expand Down
3 changes: 1 addition & 2 deletions content/browser/web_contents/web_contents_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1290,8 +1290,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
void OnDidFinishLoad(RenderFrameHostImpl* source, const GURL& url);
void OnGoToEntryAtOffset(RenderFrameHostImpl* source,
int offset,
bool has_user_gesture,
bool from_script);
bool has_user_gesture);
void OnUpdateZoomLimits(RenderViewHostImpl* source,
int minimum_percent,
int maximum_percent);
Expand Down
5 changes: 2 additions & 3 deletions content/common/frame_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -1504,10 +1504,9 @@ IPC_MESSAGE_ROUTED0(FrameHostMsg_RenderFallbackContentInParentProcess)
// Used to go to the session history entry at the given offset (ie, -1 will
// return the "back" item). This message affects a view and not just a frame,
// but is sent on the frame channel for attribution purposes.
IPC_MESSAGE_ROUTED3(FrameHostMsg_GoToEntryAtOffset,
IPC_MESSAGE_ROUTED2(FrameHostMsg_GoToEntryAtOffset,
int /* offset (from current) of history item to get */,
bool /* has_user_gesture */,
bool /* from_script */)
bool /* has_user_gesture */)

#if BUILDFLAG(USE_EXTERNAL_POPUP_MENU)

Expand Down
5 changes: 2 additions & 3 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5221,11 +5221,10 @@ void RenderFrameImpl::DidBlockNavigation(
}

void RenderFrameImpl::NavigateBackForwardSoon(int offset,
bool has_user_gesture,
bool from_script) {
bool has_user_gesture) {
render_view()->NavigateBackForwardSoon(offset, has_user_gesture);
Send(new FrameHostMsg_GoToEntryAtOffset(GetRoutingID(), offset,
has_user_gesture, from_script));
has_user_gesture));
}

base::UnguessableToken RenderFrameImpl::GetDevToolsFrameToken() {
Expand Down
4 changes: 1 addition & 3 deletions content/renderer/render_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -786,9 +786,7 @@ class CONTENT_EXPORT RenderFrameImpl
void DidBlockNavigation(const blink::WebURL& blocked_url,
const blink::WebURL& initiator_url,
blink::NavigationBlockedReason reason) override;
void NavigateBackForwardSoon(int offset,
bool has_user_gesture,
bool from_script) override;
void NavigateBackForwardSoon(int offset, bool has_user_gesture) override;
base::UnguessableToken GetDevToolsFrameToken() override;
void RenderFallbackContentInParentProcess() override;
void AbortClientNavigation() override;
Expand Down
7 changes: 2 additions & 5 deletions third_party/blink/public/web/web_local_frame_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -521,11 +521,8 @@ class BLINK_EXPORT WebLocalFrameClient {
// Tells the embedder to navigate back or forward in session history by
// the given offset (relative to the current position in session
// history). |has_user_gesture| tells whether or not this is the consequence
// of a user action. |from_script| tells whether the action was initiated from
// the execution of a script.
virtual void NavigateBackForwardSoon(int offset,
bool has_user_gesture,
bool from_script) {}
// of a user action.
virtual void NavigateBackForwardSoon(int offset, bool has_user_gesture) {}

// Returns token to be used as a frame id in the devtools protocol.
// It is derived from the content's devtools_frame_token, is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,7 @@ void LocalFrameClientImpl::LoadErrorPage(int reason) {
web_frame_->Client()->LoadErrorPage(reason);
}

bool LocalFrameClientImpl::NavigateBackForward(int offset,
bool from_script) const {
bool LocalFrameClientImpl::NavigateBackForward(int offset) const {
WebViewImpl* webview = web_frame_->ViewImpl();
DCHECK(webview->Client());
DCHECK(web_frame_->Client());
Expand All @@ -682,8 +681,7 @@ bool LocalFrameClientImpl::NavigateBackForward(int offset,

bool has_user_gesture =
LocalFrame::HasTransientUserActivation(web_frame_->GetFrame());
web_frame_->Client()->NavigateBackForwardSoon(offset, has_user_gesture,
from_script);
web_frame_->Client()->NavigateBackForwardSoon(offset, has_user_gesture);
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class LocalFrameClientImpl final : public LocalFrameClient {
void DownloadURL(const ResourceRequest&,
DownloadCrossOriginRedirects) override;
void LoadErrorPage(int reason) override;
bool NavigateBackForward(int offset, bool from_script) const override;
bool NavigateBackForward(int offset) const override;
void DidAccessInitialDocument() override;
void DidDisplayInsecureContent() override;
void DidContainInsecureFormAction() override;
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/frame/history.cc
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ void History::go(ScriptState* script_state,
return;

if (delta) {
GetFrame()->Client()->NavigateBackForward(delta, true);
GetFrame()->Client()->NavigateBackForward(delta);
} else {
// We intentionally call reload() for the current frame if delta is zero.
// Otherwise, navigation happens on the root frame.
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/frame/local_frame_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ class CORE_EXPORT LocalFrameClient : public FrameClient {
DownloadCrossOriginRedirects) = 0;
virtual void LoadErrorPage(int reason) = 0;

virtual bool NavigateBackForward(int offset, bool from_script) const = 0;
virtual bool NavigateBackForward(int offset) const = 0;

// Another page has accessed the initial empty document of this frame. It is
// no longer safe to display a provisional URL, since a URL spoof is now
Expand Down
4 changes: 1 addition & 3 deletions third_party/blink/renderer/core/loader/empty_clients.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,9 +337,7 @@ class CORE_EXPORT EmptyLocalFrameClient : public LocalFrameClient {

void TransitionToCommittedForNewPage() override {}

bool NavigateBackForward(int offset, bool from_script) const override {
return false;
}
bool NavigateBackForward(int offset) const override { return false; }
void DidDisplayInsecureContent() override {}
void DidContainInsecureFormAction() override {}
void DidRunInsecureContent(const SecurityOrigin*, const KURL&) override {}
Expand Down

0 comments on commit 043b94a

Please sign in to comment.