diff options
author | japhet@chromium.org <japhet@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-28 16:06:09 +0000 |
---|---|---|
committer | japhet@chromium.org <japhet@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-28 16:06:09 +0000 |
commit | e3b10d1ee147e27bac357b41ce7cf3b751e4a33e (patch) | |
tree | fcd5511c60fcc6eca77e30e50cd45eff11d6a9ea | |
parent | 28dfb8c039f27b27e2dd3d64b0d1cf351a15c794 (diff) | |
download | chromium_src-e3b10d1ee147e27bac357b41ce7cf3b751e4a33e.zip chromium_src-e3b10d1ee147e27bac357b41ce7cf3b751e4a33e.tar.gz chromium_src-e3b10d1ee147e27bac357b41ce7cf3b751e4a33e.tar.bz2 |
Fix pushState causing stop/reload button and favicon to flicker.
BUG=50298
Review URL: https://codereview.chromium.org/161113002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260143 0039d316-1c4b-4281-b951-d872f2087c98
33 files changed, 148 insertions, 48 deletions
diff --git a/chrome/browser/android/chrome_web_contents_delegate_android.cc b/chrome/browser/android/chrome_web_contents_delegate_android.cc index 3f6aa0e..a5c2e8c 100644 --- a/chrome/browser/android/chrome_web_contents_delegate_android.cc +++ b/chrome/browser/android/chrome_web_contents_delegate_android.cc @@ -88,9 +88,10 @@ bool RegisterChromeWebContentsDelegateAndroid(JNIEnv* env) { } void ChromeWebContentsDelegateAndroid::LoadingStateChanged( - WebContents* source) { + WebContents* source, bool to_different_document) { bool has_stopped = source == NULL || !source->IsLoading(); - WebContentsDelegateAndroid::LoadingStateChanged(source); + WebContentsDelegateAndroid::LoadingStateChanged( + source, to_different_document); LoadProgressChanged(source, has_stopped ? 1 : 0); } diff --git a/chrome/browser/android/chrome_web_contents_delegate_android.h b/chrome/browser/android/chrome_web_contents_delegate_android.h index ae915885..2b2bef1 100644 --- a/chrome/browser/android/chrome_web_contents_delegate_android.h +++ b/chrome/browser/android/chrome_web_contents_delegate_android.h @@ -37,7 +37,8 @@ class ChromeWebContentsDelegateAndroid ChromeWebContentsDelegateAndroid(JNIEnv* env, jobject obj); virtual ~ChromeWebContentsDelegateAndroid(); - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; virtual void RunFileChooser(content::WebContents* web_contents, const content::FileChooserParams& params) OVERRIDE; diff --git a/chrome/browser/chromeos/login/captive_portal_view.cc b/chrome/browser/chromeos/login/captive_portal_view.cc index 7b3c709..f20e759 100644 --- a/chrome/browser/chromeos/login/captive_portal_view.cc +++ b/chrome/browser/chromeos/login/captive_portal_view.cc @@ -84,14 +84,14 @@ void CaptivePortalView::NavigationStateChanged( // Note, |url| will be empty for "client3.google.com/generate_204" page. if (!redirected_ && url != GURL::EmptyGURL() && url != GURL(CaptivePortalStartURL())) { - DLOG(INFO) << CaptivePortalStartURL() << " vs " << url.spec(); redirected_ = true; proxy_->OnRedirected(); } } -void CaptivePortalView::LoadingStateChanged(content::WebContents* source) { - SimpleWebViewDialog::LoadingStateChanged(source); +void CaptivePortalView::LoadingStateChanged(content::WebContents* source, + bool to_different_document) { + SimpleWebViewDialog::LoadingStateChanged(source, to_different_document); // TODO(nkostylev): Fix case of no connectivity, check HTTP code returned. // Disable this heuristic as it has false positives. // Relying on just shill portal check to close dialog is fine. diff --git a/chrome/browser/chromeos/login/captive_portal_view.h b/chrome/browser/chromeos/login/captive_portal_view.h index 1a8b34e..1c8ef90 100644 --- a/chrome/browser/chromeos/login/captive_portal_view.h +++ b/chrome/browser/chromeos/login/captive_portal_view.h @@ -31,7 +31,8 @@ class CaptivePortalView : public SimpleWebViewDialog { // Overridden from content::WebContentsDelegate: virtual void NavigationStateChanged(const content::WebContents* source, unsigned changed_flags) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; private: // Contains CaptivePortalWindowProxy to be notified when redirection state is diff --git a/chrome/browser/chromeos/login/simple_web_view_dialog.cc b/chrome/browser/chromeos/login/simple_web_view_dialog.cc index 14b38de..607481b 100644 --- a/chrome/browser/chromeos/login/simple_web_view_dialog.cc +++ b/chrome/browser/chromeos/login/simple_web_view_dialog.cc @@ -265,9 +265,10 @@ void SimpleWebViewDialog::NavigationStateChanged( } } -void SimpleWebViewDialog::LoadingStateChanged(WebContents* source) { +void SimpleWebViewDialog::LoadingStateChanged(WebContents* source, + bool to_different_document) { bool is_loading = source->IsLoading(); - UpdateReload(is_loading, false); + UpdateReload(is_loading && to_different_document, false); command_updater_->UpdateCommandEnabled(IDC_STOP, is_loading); } diff --git a/chrome/browser/chromeos/login/simple_web_view_dialog.h b/chrome/browser/chromeos/login/simple_web_view_dialog.h index 69f689b..68b91f8 100644 --- a/chrome/browser/chromeos/login/simple_web_view_dialog.h +++ b/chrome/browser/chromeos/login/simple_web_view_dialog.h @@ -70,7 +70,8 @@ class SimpleWebViewDialog : public views::ButtonListener, // Implements content::WebContentsDelegate: virtual void NavigationStateChanged(const content::WebContents* source, unsigned changed_flags) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; // Implements LocationBarView::Delegate: virtual content::WebContents* GetWebContents() OVERRIDE; diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 9a802f8..85824f4 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -914,7 +914,7 @@ void Browser::TabInsertedAt(WebContents* contents, // Make sure the loading state is updated correctly, otherwise the throbber // won't start if the page is loading. - LoadingStateChanged(contents); + LoadingStateChanged(contents, true); interstitial_observers_.push_back(new InterstitialObserver(this, contents)); @@ -1339,13 +1339,14 @@ void Browser::DeactivateContents(WebContents* contents) { window_->Deactivate(); } -void Browser::LoadingStateChanged(WebContents* source) { +void Browser::LoadingStateChanged(WebContents* source, + bool to_different_document) { window_->UpdateLoadingAnimations(tab_strip_model_->TabsAreLoading()); window_->UpdateTitleBar(); WebContents* selected_contents = tab_strip_model_->GetActiveWebContents(); if (source == selected_contents) { - bool is_loading = source->IsLoading(); + bool is_loading = source->IsLoading() && to_different_document; command_controller_->LoadingStateChanged(is_loading, false); if (GetStatusBubble()) { GetStatusBubble()->SetStatus(CoreTabHelper::FromWebContents( diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h index d66ac0d..0e6d0bb 100644 --- a/chrome/browser/ui/browser.h +++ b/chrome/browser/ui/browser.h @@ -549,7 +549,8 @@ class Browser : public TabStripModelObserver, bool* was_blocked) OVERRIDE; virtual void ActivateContents(content::WebContents* contents) OVERRIDE; virtual void DeactivateContents(content::WebContents* contents) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; virtual void CloseContents(content::WebContents* source) OVERRIDE; virtual void MoveContents(content::WebContents* source, const gfx::Rect& pos) OVERRIDE; diff --git a/chrome/browser/ui/browser_tab_strip_model_delegate.cc b/chrome/browser/ui/browser_tab_strip_model_delegate.cc index ab08564..730a2ad 100644 --- a/chrome/browser/ui/browser_tab_strip_model_delegate.cc +++ b/chrome/browser/ui/browser_tab_strip_model_delegate.cc @@ -77,7 +77,7 @@ Browser* BrowserTabStripModelDelegate::CreateNewStripWithContents( // won't start if the page is loading. // TODO(beng): find a better way of doing this. static_cast<content::WebContentsDelegate*>(browser)-> - LoadingStateChanged(item.web_contents); + LoadingStateChanged(item.web_contents, true); } return browser; diff --git a/chrome/browser/ui/cocoa/web_dialog_window_controller.mm b/chrome/browser/ui/cocoa/web_dialog_window_controller.mm index 3e61033..db0be89 100644 --- a/chrome/browser/ui/cocoa/web_dialog_window_controller.mm +++ b/chrome/browser/ui/cocoa/web_dialog_window_controller.mm @@ -73,7 +73,8 @@ public: const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; private: WebDialogWindowController* controller_; // weak @@ -240,7 +241,7 @@ void WebDialogWindowDelegateBridge::AddNewContents( } void WebDialogWindowDelegateBridge::LoadingStateChanged( - content::WebContents* source) { + content::WebContents* source, bool to_different_document) { if (delegate_) delegate_->OnLoadingStateChanged(source); } diff --git a/chrome/browser/ui/panels/panel_host.cc b/chrome/browser/ui/panels/panel_host.cc index 89c1863..9d8bbd3 100644 --- a/chrome/browser/ui/panels/panel_host.cc +++ b/chrome/browser/ui/panels/panel_host.cc @@ -160,8 +160,9 @@ void PanelHost::DeactivateContents(content::WebContents* contents) { panel_->Deactivate(); } -void PanelHost::LoadingStateChanged(content::WebContents* source) { - bool is_loading = source->IsLoading(); +void PanelHost::LoadingStateChanged(content::WebContents* source, + bool to_different_document) { + bool is_loading = source->IsLoading() && to_different_document; panel_->LoadingStateChanged(is_loading); } diff --git a/chrome/browser/ui/panels/panel_host.h b/chrome/browser/ui/panels/panel_host.h index 37ab05b..d383f63 100644 --- a/chrome/browser/ui/panels/panel_host.h +++ b/chrome/browser/ui/panels/panel_host.h @@ -61,7 +61,8 @@ class PanelHost : public content::WebContentsDelegate, bool* was_blocked) OVERRIDE; virtual void ActivateContents(content::WebContents* contents) OVERRIDE; virtual void DeactivateContents(content::WebContents* contents) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; virtual void CloseContents(content::WebContents* source) OVERRIDE; virtual void MoveContents(content::WebContents* source, const gfx::Rect& pos) OVERRIDE; diff --git a/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc b/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc index 2caef65..767db59 100644 --- a/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc +++ b/chrome/browser/ui/webui/web_dialog_web_contents_delegate_unittest.cc @@ -69,7 +69,7 @@ TEST_F(WebDialogWebContentsDelegateTest, DoNothingMethodsTest) { content::PAGE_TRANSITION_TYPED, history::SOURCE_SYNCED, false); test_web_contents_delegate_->NavigationStateChanged(NULL, 0); test_web_contents_delegate_->ActivateContents(NULL); - test_web_contents_delegate_->LoadingStateChanged(NULL); + test_web_contents_delegate_->LoadingStateChanged(NULL, true); test_web_contents_delegate_->CloseContents(NULL); test_web_contents_delegate_->UpdateTargetURL(NULL, 0, GURL()); test_web_contents_delegate_->MoveContents(NULL, gfx::Rect()); diff --git a/components/web_contents_delegate_android/web_contents_delegate_android.cc b/components/web_contents_delegate_android/web_contents_delegate_android.cc index d12a23f..a4f8213 100644 --- a/components/web_contents_delegate_android/web_contents_delegate_android.cc +++ b/components/web_contents_delegate_android/web_contents_delegate_android.cc @@ -139,7 +139,8 @@ void WebContentsDelegateAndroid::DeactivateContents(WebContents* contents) { // to focus. Not implemented on Android. } -void WebContentsDelegateAndroid::LoadingStateChanged(WebContents* source) { +void WebContentsDelegateAndroid::LoadingStateChanged(WebContents* source, + bool to_different_document) { JNIEnv* env = AttachCurrentThread(); ScopedJavaLocalRef<jobject> obj = GetJavaDelegate(env); if (obj.is_null()) diff --git a/components/web_contents_delegate_android/web_contents_delegate_android.h b/components/web_contents_delegate_android/web_contents_delegate_android.h index d6720280..4c3568a 100644 --- a/components/web_contents_delegate_android/web_contents_delegate_android.h +++ b/components/web_contents_delegate_android/web_contents_delegate_android.h @@ -62,7 +62,8 @@ class WebContentsDelegateAndroid : public content::WebContentsDelegate { unsigned changed_flags) OVERRIDE; virtual void ActivateContents(content::WebContents* contents) OVERRIDE; virtual void DeactivateContents(content::WebContents* contents) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; virtual void LoadProgressChanged(content::WebContents* source, double load_progress) OVERRIDE; virtual void RendererUnresponsive(content::WebContents* source) OVERRIDE; diff --git a/content/browser/frame_host/interstitial_page_impl.cc b/content/browser/frame_host/interstitial_page_impl.cc index 885d57b..fd93639 100644 --- a/content/browser/frame_host/interstitial_page_impl.cc +++ b/content/browser/frame_host/interstitial_page_impl.cc @@ -458,7 +458,7 @@ void InterstitialPageImpl::DidNavigate( // an interstitial would hang. web_contents_was_loading_ = controller_->delegate()->IsLoading(); controller_->delegate()->SetIsLoading( - controller_->delegate()->GetRenderViewHost(), false, NULL); + controller_->delegate()->GetRenderViewHost(), false, true, NULL); } void InterstitialPageImpl::UpdateTitle( @@ -603,7 +603,7 @@ void InterstitialPageImpl::Proceed() { // Resumes the throbber, if applicable. if (web_contents_was_loading_) controller_->delegate()->SetIsLoading( - controller_->delegate()->GetRenderViewHost(), true, NULL); + controller_->delegate()->GetRenderViewHost(), true, true, NULL); // If this is a new navigation, the old page is going away, so we cancel any // blocked requests for it. If it is not a new navigation, then it means the diff --git a/content/browser/frame_host/navigation_controller_delegate.h b/content/browser/frame_host/navigation_controller_delegate.h index 404e927..77bc622 100644 --- a/content/browser/frame_host/navigation_controller_delegate.h +++ b/content/browser/frame_host/navigation_controller_delegate.h @@ -71,6 +71,7 @@ class NavigationControllerDelegate { virtual void DetachInterstitialPage() = 0; virtual void SetIsLoading(RenderViewHost* render_view_host, bool is_loading, + bool to_different_document, LoadNotificationDetails* details) = 0; }; diff --git a/content/browser/frame_host/render_frame_host_delegate.h b/content/browser/frame_host/render_frame_host_delegate.h index b6ebfa9..f5e00ab 100644 --- a/content/browser/frame_host/render_frame_host_delegate.h +++ b/content/browser/frame_host/render_frame_host_delegate.h @@ -32,7 +32,10 @@ class CONTENT_EXPORT RenderFrameHostDelegate { // The top-level RenderFrame began loading a new page. This corresponds to // Blink's notion of the throbber starting. - virtual void DidStartLoading(RenderFrameHost* render_frame_host) {} + // |to_different_document| will be true unless the load is a fragment + // navigation, or triggered by history.pushState/replaceState. + virtual void DidStartLoading(RenderFrameHost* render_frame_host, + bool to_different_document) {} // The top-level RenderFrame stopped loading a page. This corresponds to // Blink's notion of the throbber stopping. diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 147dd62..bf08aff 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -505,8 +505,8 @@ void RenderFrameHostImpl::SwapOut() { } } -void RenderFrameHostImpl::OnDidStartLoading() { - delegate_->DidStartLoading(this); +void RenderFrameHostImpl::OnDidStartLoading(bool to_different_document) { + delegate_->DidStartLoading(this, to_different_document); } void RenderFrameHostImpl::OnDidStopLoading() { @@ -664,7 +664,7 @@ void RenderFrameHostImpl::Navigate(const FrameMsg_Navigate_Params& params) { // Blink doesn't send throb notifications for JavaScript URLs, so we // don't want to either. if (!params.url.SchemeIs(kJavaScriptScheme)) - delegate_->DidStartLoading(this); + delegate_->DidStartLoading(this, true); } void RenderFrameHostImpl::NavigateToURL(const GURL& url) { diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 5d4951a..483783e 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h @@ -140,7 +140,9 @@ class CONTENT_EXPORT RenderFrameHostImpl : public RenderFrameHost { // TODO(nasko): This method is public so RenderViewHostImpl::Navigate can // call it directly. It should be made private once Navigate moves here. - void OnDidStartLoading(); + // |to_different_document| will be true unless the load is a fragment + // navigation, or triggered by history.pushState/replaceState. + void OnDidStartLoading(bool to_different_document); // Sends the given navigation message. Use this rather than sending it // yourself since this does the internal bookkeeping described below. This diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 195edc3..38b91604 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -2614,6 +2614,7 @@ void WebContentsImpl::ActivateAndShowRepostFormWarningDialog() { // loading, or done loading. void WebContentsImpl::SetIsLoading(RenderViewHost* render_view_host, bool is_loading, + bool to_different_document, LoadNotificationDetails* details) { if (is_loading == is_loading_) return; @@ -2632,7 +2633,7 @@ void WebContentsImpl::SetIsLoading(RenderViewHost* render_view_host, waiting_for_response_ = is_loading; if (delegate_) - delegate_->LoadingStateChanged(this); + delegate_->LoadingStateChanged(this, to_different_document); NotifyNavigationStateChanged(INVALIDATE_TYPE_LOAD); std::string url = (details ? details->url.possibly_invalid_spec() : "NULL"); @@ -2902,7 +2903,7 @@ void WebContentsImpl::RenderViewTerminated(RenderViewHost* rvh, dialog_manager_->CancelActiveAndPendingDialogs(this); ClearPowerSaveBlockers(rvh); - SetIsLoading(rvh, false, NULL); + SetIsLoading(rvh, false, true, NULL); NotifyDisconnected(); SetIsCrashed(status, error_code); GetView()->OnTabCrashed(GetCrashedStatus(), crashed_error_code_); @@ -3031,8 +3032,10 @@ void WebContentsImpl::RequestMove(const gfx::Rect& new_bounds) { delegate_->MoveContents(this, new_bounds); } -void WebContentsImpl::DidStartLoading(RenderFrameHost* render_frame_host) { - SetIsLoading(render_frame_host->GetRenderViewHost(), true, NULL); +void WebContentsImpl::DidStartLoading(RenderFrameHost* render_frame_host, + bool to_different_document) { + SetIsLoading(render_frame_host->GetRenderViewHost(), true, + to_different_document, NULL); } void WebContentsImpl::DidStopLoading(RenderFrameHost* render_frame_host) { @@ -3057,7 +3060,8 @@ void WebContentsImpl::DidStopLoading(RenderFrameHost* render_frame_host) { controller_.GetCurrentEntryIndex())); } - SetIsLoading(render_frame_host->GetRenderViewHost(), false, details.get()); + SetIsLoading(render_frame_host->GetRenderViewHost(), false, true, + details.get()); } void WebContentsImpl::DidCancelLoading() { diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h index e6135d1..ae72bdf 100644 --- a/content/browser/web_contents/web_contents_impl.h +++ b/content/browser/web_contents/web_contents_impl.h @@ -304,7 +304,8 @@ class CONTENT_EXPORT WebContentsImpl const IPC::Message& message) OVERRIDE; virtual void RenderFrameCreated(RenderFrameHost* render_frame_host) OVERRIDE; virtual void RenderFrameDeleted(RenderFrameHost* render_frame_host) OVERRIDE; - virtual void DidStartLoading(RenderFrameHost* render_frame_host) OVERRIDE; + virtual void DidStartLoading(RenderFrameHost* render_frame_host, + bool to_different_document) OVERRIDE; virtual void DidStopLoading(RenderFrameHost* render_frame_host) OVERRIDE; virtual void WorkerCrashed(RenderFrameHost* render_frame_host) OVERRIDE; virtual void ShowContextMenu(RenderFrameHost* render_frame_host, @@ -589,6 +590,7 @@ class CONTENT_EXPORT WebContentsImpl // (but can be null if not applicable). virtual void SetIsLoading(RenderViewHost* render_view_host, bool is_loading, + bool to_different_document, LoadNotificationDetails* details) OVERRIDE; typedef base::Callback<void(WebContents*)> CreatedCallback; diff --git a/content/browser/web_contents/web_contents_impl_browsertest.cc b/content/browser/web_contents/web_contents_impl_browsertest.cc index 0754799..3949928 100644 --- a/content/browser/web_contents/web_contents_impl_browsertest.cc +++ b/content/browser/web_contents/web_contents_impl_browsertest.cc @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/strings/utf_string_conversions.h" #include "base/values.h" #include "content/browser/frame_host/navigation_entry_impl.h" #include "content/browser/web_contents/web_contents_impl.h" @@ -149,6 +150,31 @@ class RenderViewSizeObserver : public WebContentsObserver { gfx::Size rwhv_create_size_; }; +class LoadingStateChangedDelegate : public WebContentsDelegate { + public: + LoadingStateChangedDelegate() + : loadingStateChangedCount_(0) + , loadingStateToDifferentDocumentCount_(0) { + } + + // WebContentsDelgate: + virtual void LoadingStateChanged(WebContents* contents, + bool to_different_document) OVERRIDE { + loadingStateChangedCount_++; + if (to_different_document) + loadingStateToDifferentDocumentCount_++; + } + + int loadingStateChangedCount() const { return loadingStateChangedCount_; } + int loadingStateToDifferentDocumentCount() const { + return loadingStateToDifferentDocumentCount_; + } + + private: + int loadingStateChangedCount_; + int loadingStateToDifferentDocumentCount_; +}; + // See: http://crbug.com/298193 #if defined(OS_WIN) #define MAYBE_DidStopLoadingDetails DISABLED_DidStopLoadingDetails @@ -387,4 +413,29 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, EXPECT_EQ(observer.last_rfh(), shell()->web_contents()->GetMainFrame()); } +IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, + LoadingStateChangedForSameDocumentNavigation) { + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + scoped_ptr<LoadingStateChangedDelegate> delegate( + new LoadingStateChangedDelegate()); + shell()->web_contents()->SetDelegate(delegate.get()); + + LoadStopNotificationObserver load_observer( + &shell()->web_contents()->GetController()); + TitleWatcher title_watcher(shell()->web_contents(), + base::ASCIIToUTF16("pushState")); + NavigateToURL(shell(), embedded_test_server()->GetURL("/push_state.html")); + load_observer.Wait(); + base::string16 title = title_watcher.WaitAndGetTitle(); + ASSERT_EQ(title, base::ASCIIToUTF16("pushState")); + + // LoadingStateChanged should be called 4 times: start and stop for the + // initial load of push_state.html, and start and stop for the "navigation" + // triggered by history.pushState(). However, the start notification for the + // history.pushState() navigation should set to_different_document to false. + EXPECT_EQ("pushState", shell()->web_contents()->GetURL().ref()); + EXPECT_EQ(4, delegate->loadingStateChangedCount()); + EXPECT_EQ(3, delegate->loadingStateToDifferentDocumentCount()); +} + } // namespace content diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h index d5fc7ca..c64490b 100644 --- a/content/common/frame_messages.h +++ b/content/common/frame_messages.h @@ -385,7 +385,10 @@ IPC_MESSAGE_ROUTED3(FrameHostMsg_DidFailLoadWithError, // Sent when the renderer starts loading the page. This corresponds to // Blink's notion of the throbber starting. Note that sometimes you may get // duplicates of these during a single load. -IPC_MESSAGE_ROUTED0(FrameHostMsg_DidStartLoading) +// |to_different_document| will be true unless the load is a fragment +// navigation, or triggered by history.pushState/replaceState. +IPC_MESSAGE_ROUTED1(FrameHostMsg_DidStartLoading, + bool /* to_different_document */) // Sent when the renderer is done loading a page. This corresponds to Blink's // notion of the throbber stopping. diff --git a/content/public/browser/web_contents_delegate.h b/content/public/browser/web_contents_delegate.h index 2dd2418..b869a52 100644 --- a/content/public/browser/web_contents_delegate.h +++ b/content/public/browser/web_contents_delegate.h @@ -112,7 +112,10 @@ class CONTENT_EXPORT WebContentsDelegate { // Notifies the delegate that this contents is starting or is done loading // some resource. The delegate should use this notification to represent // loading feedback. See WebContents::IsLoading() - virtual void LoadingStateChanged(WebContents* source) {} + // |to_different_document| will be true unless the load is a fragment + // navigation, or triggered by history.pushState/replaceState. + virtual void LoadingStateChanged(WebContents* source, + bool to_different_document) {} // Notifies the delegate that the page has made some progress loading. // |progress| is a value between 0.0 (nothing loaded) to 1.0 (page fully diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index bfed42d..6ec073c 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -2603,7 +2603,7 @@ void RenderFrameImpl::didStartLoading(bool to_different_document) { render_view_->FrameDidStartLoading(frame_); if (!view_was_loading) - Send(new FrameHostMsg_DidStartLoading(routing_id_)); + Send(new FrameHostMsg_DidStartLoading(routing_id_, to_different_document)); } void RenderFrameImpl::didStopLoading() { diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index 3295358..a032b57 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h @@ -101,6 +101,8 @@ class CONTENT_EXPORT RenderFrameImpl // TODO(nasko): Those are page-level methods at this time and come from // WebViewClient. We should move them to be WebFrameClient calls and put // logic in the browser side to balance starts/stops. + // |to_different_document| will be true unless the load is a fragment + // navigation, or triggered by history.pushState/replaceState. virtual void didStartLoading(bool to_different_document); virtual void didStopLoading(); virtual void didChangeLoadProgress(double load_progress); diff --git a/content/shell/browser/shell.cc b/content/shell/browser/shell.cc index 6549123..b19ad1f 100644 --- a/content/shell/browser/shell.cc +++ b/content/shell/browser/shell.cc @@ -205,13 +205,14 @@ void Shell::Stop() { web_contents_->GetView()->Focus(); } -void Shell::UpdateNavigationControls() { +void Shell::UpdateNavigationControls(bool to_different_document) { int current_index = web_contents_->GetController().GetCurrentEntryIndex(); int max_index = web_contents_->GetController().GetEntryCount() - 1; PlatformEnableUIControl(BACK_BUTTON, current_index > 0); PlatformEnableUIControl(FORWARD_BUTTON, current_index < max_index); - PlatformEnableUIControl(STOP_BUTTON, web_contents_->IsLoading()); + PlatformEnableUIControl(STOP_BUTTON, + to_different_document && web_contents_->IsLoading()); } void Shell::ShowDevTools() { @@ -266,8 +267,9 @@ WebContents* Shell::OpenURLFromTab(WebContents* source, return source; } -void Shell::LoadingStateChanged(WebContents* source) { - UpdateNavigationControls(); +void Shell::LoadingStateChanged(WebContents* source, + bool to_different_document) { + UpdateNavigationControls(to_different_document); PlatformSetIsLoading(source->IsLoading()); } diff --git a/content/shell/browser/shell.h b/content/shell/browser/shell.h index 5248199..1156f21 100644 --- a/content/shell/browser/shell.h +++ b/content/shell/browser/shell.h @@ -65,7 +65,7 @@ class Shell : public WebContentsDelegate, void GoBackOrForward(int offset); void Reload(); void Stop(); - void UpdateNavigationControls(); + void UpdateNavigationControls(bool to_different_document); void Close(); void ShowDevTools(); void ShowDevToolsForElementAt(int x, int y); @@ -119,7 +119,8 @@ class Shell : public WebContentsDelegate, const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked) OVERRIDE; - virtual void LoadingStateChanged(WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(WebContents* source, + bool to_different_document) OVERRIDE; #if defined(OS_ANDROID) virtual void LoadProgressChanged(WebContents* source, double progress) OVERRIDE; diff --git a/content/test/data/push_state.html b/content/test/data/push_state.html new file mode 100644 index 0000000..22178ee --- /dev/null +++ b/content/test/data/push_state.html @@ -0,0 +1,12 @@ +<html> +<head> +<script> +window.onload = function() { + setTimeout(function() { + history.pushState("", "", "#pushState"); + document.title = "pushState"; + }, 0); +} +</script> +</head> +</html> diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc index 900c850..55cab77 100644 --- a/content/test/test_web_contents.cc +++ b/content/test/test_web_contents.cc @@ -129,7 +129,7 @@ void TestWebContents::NavigateAndCommit(const GURL& url) { } void TestWebContents::TestSetIsLoading(bool value) { - SetIsLoading(GetRenderViewHost(), value, NULL); + SetIsLoading(GetRenderViewHost(), value, true, NULL); } void TestWebContents::CommitPendingNavigation() { diff --git a/ui/views/controls/webview/web_dialog_view.cc b/ui/views/controls/webview/web_dialog_view.cc index 98da5ad..949c82a 100644 --- a/ui/views/controls/webview/web_dialog_view.cc +++ b/ui/views/controls/webview/web_dialog_view.cc @@ -324,7 +324,8 @@ void WebDialogView::AddNewContents(content::WebContents* source, was_blocked); } -void WebDialogView::LoadingStateChanged(content::WebContents* source) { +void WebDialogView::LoadingStateChanged(content::WebContents* source, + bool to_different_document) { if (delegate_) delegate_->OnLoadingStateChanged(source); } diff --git a/ui/views/controls/webview/web_dialog_view.h b/ui/views/controls/webview/web_dialog_view.h index 51d67eb..902aa32 100644 --- a/ui/views/controls/webview/web_dialog_view.h +++ b/ui/views/controls/webview/web_dialog_view.h @@ -110,7 +110,8 @@ class WEBVIEW_EXPORT WebDialogView : public views::ClientView, const gfx::Rect& initial_pos, bool user_gesture, bool* was_blocked) OVERRIDE; - virtual void LoadingStateChanged(content::WebContents* source) OVERRIDE; + virtual void LoadingStateChanged(content::WebContents* source, + bool to_different_document) OVERRIDE; virtual void BeforeUnloadFired(content::WebContents* tab, bool proceed, bool* proceed_to_fire_unload) OVERRIDE; |