diff options
Diffstat (limited to 'chrome/browser')
21 files changed, 892 insertions, 713 deletions
diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index 6a117c7..1456adc 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -572,6 +572,23 @@ class DocumentPrintedNotificationObserver : public NotificationObserver { bool success_; }; +class AutomationInterstitialPage : public InterstitialPage { + public: + AutomationInterstitialPage(TabContents* tab, + const GURL& url, + const std::string& contents) + : InterstitialPage(tab, true, url), + contents_(contents) { + } + + virtual std::string GetHTMLContents() { return contents_; } + + private: + std::string contents_; + + DISALLOW_COPY_AND_ASSIGN(AutomationInterstitialPage); +}; + AutomationProvider::AutomationProvider(Profile* profile) : redirect_query_(0), profile_(profile) { @@ -1906,7 +1923,11 @@ void AutomationProvider::ShowInterstitialPage(const IPC::Message& message, true), NULL); WebContents* web_contents = tab_contents->AsWebContents(); - web_contents->ShowInterstitialPage(html_text, NULL); + AutomationInterstitialPage* interstitial = + new AutomationInterstitialPage(web_contents, + GURL("about:interstitial"), + html_text); + web_contents->ShowInterstitialPage(interstitial); return; } } @@ -2076,8 +2097,8 @@ void AutomationProvider::ActionOnSSLBlockingPage(const IPC::Message& message, NavigationEntry* entry = tab->GetActiveEntry(); if (entry->page_type() == NavigationEntry::INTERSTITIAL_PAGE) { TabContents* tab_contents = tab->GetTabContents(TAB_CONTENTS_WEB); - SSLBlockingPage* ssl_blocking_page = - SSLBlockingPage::GetSSLBlockingPage(tab_contents); + InterstitialPage* ssl_blocking_page = + InterstitialPage::GetInterstitialPage(tab_contents); if (ssl_blocking_page) { if (proceed) { AddNavigationStatusListener(tab, diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj index e437446..8a83569 100644 --- a/chrome/browser/browser.vcproj +++ b/chrome/browser/browser.vcproj @@ -2143,7 +2143,11 @@ > </File> <File - RelativePath=".\interstitial_page_delegate.h" + RelativePath=".\interstitial_page.cc" + > + </File> + <File + RelativePath=".\interstitial_page.h" > </File> <File diff --git a/chrome/browser/browser_commands.cc b/chrome/browser/browser_commands.cc index e4e5938d..12d70ce 100644 --- a/chrome/browser/browser_commands.cc +++ b/chrome/browser/browser_commands.cc @@ -18,7 +18,7 @@ #include "chrome/browser/debugger/debugger_window.h" #include "chrome/browser/views/download_tab_view.h" #include "chrome/browser/history_tab_ui.h" -#include "chrome/browser/interstitial_page_delegate.h" +#include "chrome/browser/interstitial_page.h" #include "chrome/browser/navigation_entry.h" #include "chrome/browser/options_window.h" #include "chrome/browser/tab_restore_service.h" @@ -732,35 +732,10 @@ void Browser::GoBack() { TabContents* current_tab = GetSelectedTabContents(); if (current_tab) { WebContents* web_contents = current_tab->AsWebContents(); - // If we are showing an interstitial page, we don't need to navigate back - // to the previous page as it is already available in a render view host - // of the WebContents. This makes the back more snappy and avoids potential - // reloading of POST pages. if (web_contents && web_contents->showing_interstitial_page()) { - // Let the delegate decide (if any) if it wants to handle the back - // navigation (it may have extra things to do). - if (web_contents->interstitial_page_delegate() && - web_contents->interstitial_page_delegate()->GoBack()) { - return; - } - // TODO(jcampan): #1283764 once we refactored and only use the - // interstitial delegate, the code below should go away. - NavigationEntry* prev_nav_entry = web_contents->controller()-> - GetEntryAtOffset(-1); - DCHECK(prev_nav_entry); - // We do a normal back if: - // - the page is not a WebContents, its TabContents might have to be - // recreated. - // - we have not yet visited that navigation entry (typically session - // restore), in which case the page is not already available. - if (prev_nav_entry->tab_type() == TAB_CONTENTS_WEB && - !prev_nav_entry->restored()) { - // It is the job of the code that shows the interstitial to listen for - // notifications of the interstitial getting hidden and appropriately - // dealing with the navigation entries. - web_contents->HideInterstitialPage(false, false); - return; - } + // Pressing back on an interstitial page means "don't proceed". + web_contents->interstitial_page()->DontProceed(); + return; } } NavigationController* nc = GetSelectedNavigationController(); diff --git a/chrome/browser/interstitial_page.cc b/chrome/browser/interstitial_page.cc new file mode 100644 index 0000000..5cbc37e --- /dev/null +++ b/chrome/browser/interstitial_page.cc @@ -0,0 +1,116 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/interstitial_page.h" + +#include "chrome/browser/browser.h" +#include "chrome/browser/browser_resources.h" +#include "chrome/browser/dom_operation_notification_details.h" +#include "chrome/browser/navigation_controller.h" +#include "chrome/browser/navigation_entry.h" +#include "chrome/browser/tab_contents.h" +#include "chrome/browser/web_contents.h" + +// static +InterstitialPage::InterstitialPageMap* + InterstitialPage::tab_to_interstitial_page_ = NULL; + +InterstitialPage::InterstitialPage(TabContents* tab, + bool create_navigation_entry, + const GURL& url) + : tab_(tab), + url_(url), + delegate_has_been_notified_(false), + create_navigation_entry_(create_navigation_entry) { + InitInterstitialPageMap(); + + // If there's already an interstitial in this tab, then we're about to + // replace it. We should be ok with just deleting the previous + // InterstitialPage (not hiding it first), since we're about to be shown. + InterstitialPageMap::const_iterator iter = + tab_to_interstitial_page_->find(tab_); + if (iter != tab_to_interstitial_page_->end()) { + // Deleting the InterstitialPage will also remove it from the map. + delete iter->second; + } + (*tab_to_interstitial_page_)[tab_] = this; + + // Register for DOM operations, this is how the page notifies us of the user + // selection. + notification_registrar_.Add(this, NOTIFY_DOM_OPERATION_RESPONSE, + Source<TabContents>(tab_)); +} + +InterstitialPage::~InterstitialPage() { + InterstitialPageMap::iterator iter = tab_to_interstitial_page_->find(tab_); + DCHECK(iter != tab_to_interstitial_page_->end()); + tab_to_interstitial_page_->erase(iter); +} + +void InterstitialPage::Show() { + DCHECK(tab_->type() == TAB_CONTENTS_WEB); + WebContents* tab = tab_->AsWebContents(); + + if (create_navigation_entry_) { + NavigationEntry* entry = new NavigationEntry(TAB_CONTENTS_WEB); + entry->set_url(url_); + entry->set_display_url(url_); + entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE); + + // Give sub-classes a chance to set some states on the navigation entry. + UpdateEntry(entry); + + tab_->controller()->AddTransientEntry(entry); + } + + tab->ShowInterstitialPage(this); +} + +void InterstitialPage::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + DCHECK(type == NOTIFY_DOM_OPERATION_RESPONSE); + std::string json = Details<DomOperationNotificationDetails>(details)->json(); + CommandReceived(json); +} + +void InterstitialPage::InterstitialClosed() {
+ delete this;
+}
+ +void InterstitialPage::Proceed() { + DCHECK(tab_->type() == TAB_CONTENTS_WEB); + tab_->AsWebContents()->HideInterstitialPage(true, true); +} + +void InterstitialPage::DontProceed() { + if (create_navigation_entry_) { + // Since no navigation happens we have to discard the transient entry + // explicitely. Note that by calling DiscardNonCommittedEntries() we also + // discard the pending entry, which is what we want, since the navigation is + // cancelled. + tab_->controller()->DiscardNonCommittedEntries(); + } + tab_->AsWebContents()->HideInterstitialPage(false, false); + + // WARNING: we are now deleted! +} + +// static +void InterstitialPage::InitInterstitialPageMap() { + if (!tab_to_interstitial_page_) + tab_to_interstitial_page_ = new InterstitialPageMap; +} + +// static +InterstitialPage* InterstitialPage::GetInterstitialPage( + TabContents* tab_contents) { + InitInterstitialPageMap(); + InterstitialPageMap::const_iterator iter = + tab_to_interstitial_page_->find(tab_contents); + if (iter == tab_to_interstitial_page_->end()) + return NULL; + + return iter->second; +}
diff --git a/chrome/browser/interstitial_page.h b/chrome/browser/interstitial_page.h new file mode 100644 index 0000000..ce17818 --- /dev/null +++ b/chrome/browser/interstitial_page.h @@ -0,0 +1,118 @@ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_INTERSTITIAL_PAGE_H_ +#define CHROME_BROWSER_INTERSTITIAL_PAGE_H_ + +#include <string> + +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_service.h" +#include "googleurl/src/gurl.h" + +class NavigationEntry; +class TabContents; + +// This class is a base class for interstitial pages, pages that show some +// informative message asking for user validation before reaching the target +// page. (Navigating to a page served over bad HTTPS or a page contining +// malware are typical cases where an interstitial is required.) +// +// If specified in its constructor, this class creates a navigation entry so +// that when the interstitial shows, the current entry is the target URL. +// +// InterstitialPage instances take care of deleting themselves when closed +// through a navigation, the WebContents closing them or the tab containing them +// being closed. + +class InterstitialPage : public NotificationObserver { + public: + // Creates an interstitial page to show in |tab|. If |create_navigation_entry| + // is true, a temporary navigation entry is created with the URL |url| and + // added to the navigation controller (so the interstitial page appears as a + // new navigation entry). + InterstitialPage(TabContents* tab, + bool create_navigation_entry, + const GURL& url); + virtual ~InterstitialPage(); + + // Shows the interstitial page in the tab. + void Show(); + + // Invoked by the tab showing the interstitial to notify that the interstitial
+ // page was closed.
+ virtual void InterstitialClosed();
+ + // Retrieves the InterstitialPage if any associated with the specified + // |tab_contents| (used by ui tests). + static InterstitialPage* GetInterstitialPage(TabContents* tab_contents); + + // Sub-classes should return the HTML that should be displayed in the page. + virtual std::string GetHTMLContents() { return std::string(); } + + // Reverts to the page showing before the interstitial. + // Sub-classes should call this method when the user has chosen NOT to proceed + // to the target URL. + // Warning: 'this' has been deleted when this method returns. + virtual void DontProceed(); + + protected: + // Invoked when the page sent a command through DOMAutomation. + virtual void CommandReceived(const std::string& command) { } + + // Invoked with the NavigationEntry that is going to be added to the + // navigation controller. + // Gives an opportunity to sub-classes to set states on the |entry|. + // Note that this is only called if the InterstitialPage was constructed with + // |create_navigation_entry| set to true. + virtual void UpdateEntry(NavigationEntry* entry) { } + + // Sub-classes should call this method when the user has chosen to proceed to + // the target URL. + // Warning: 'this' has been deleted when this method returns. + virtual void Proceed(); + + TabContents* tab() const { return tab_; } + const GURL& url() const { return url_; } + + private: + // AutomationProvider needs access to Proceed and DontProceed to simulate + // user actions. + friend class AutomationProvider; + + // NotificationObserver method. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + + // Initializes tab_to_interstitial_page_ in a thread-safe manner. + // Should be called before accessing tab_to_interstitial_page_. + static void InitInterstitialPageMap(); + + // A flag to indicate if we've notified |delegate_| of the user's decision. + bool delegate_has_been_notified_; + + // The tab in which we are displayed. + TabContents* tab_; + + // The URL that is shown when the interstitial is showing. + GURL url_; + + // Whether a transient navigation entry should be created when the page is + // shown. + bool create_navigation_entry_; + + // Notification magic. + NotificationRegistrar notification_registrar_; + + // We keep a map of the various blocking pages shown as the UI tests need to + // be able to retrieve them. + typedef std::map<TabContents*,InterstitialPage*> InterstitialPageMap; + static InterstitialPageMap* tab_to_interstitial_page_; + + DISALLOW_COPY_AND_ASSIGN(InterstitialPage); +}; + +#endif // #ifndef CHROME_BROWSER_INTERSTITIAL_PAGE_H_ +
diff --git a/chrome/browser/interstitial_page_delegate.h b/chrome/browser/interstitial_page_delegate.h deleted file mode 100644 index 9032924..0000000 --- a/chrome/browser/interstitial_page_delegate.h +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_INTERSTITIAL_PAGE_DELEGATE_H__ -#define CHROME_BROWSER_INTERSTITIAL_PAGE_DELEGATE_H__ - -// The InterstitialPageDelegate interface allows implementing classes to take -// action when different events happen while an interstitial page is shown. -// It is passed to the WebContents::ShowInterstitial() method. - -class InterstitialPageDelegate { - public: - // Notification that the interstitial page was closed. This is the last call - // that the delegate gets. - virtual void InterstitialClosed() = 0; - - // The tab showing this interstitial page is navigating back. If this returns - // false, the default back behavior is executed (navigating to the previous - // navigation entry). If this returns true, no navigation is performed (it - // is assumed the implementation took care of the navigation). - virtual bool GoBack() = 0; -}; - -#endif // CHROME_BROWSER_INTERSTITIAL_PAGE_DELEGATE_H__ - - diff --git a/chrome/browser/navigation_controller.cc b/chrome/browser/navigation_controller.cc index e8908fe..2e7b3cc 100644 --- a/chrome/browser/navigation_controller.cc +++ b/chrome/browser/navigation_controller.cc @@ -168,6 +168,7 @@ NavigationController::NavigationController(TabContents* contents, pending_entry_(NULL), last_committed_entry_index_(-1), pending_entry_index_(-1), + transient_entry_index_(-1), active_contents_(contents), max_restored_page_id_(-1), ssl_manager_(this, NULL), @@ -187,6 +188,7 @@ NavigationController::NavigationController( pending_entry_(NULL), last_committed_entry_index_(-1), pending_entry_index_(-1), + transient_entry_index_(-1), active_contents_(NULL), max_restored_page_id_(-1), ssl_manager_(this, NULL), @@ -207,7 +209,7 @@ NavigationController::~NavigationController() { DCHECK(tab_contents_map_.empty()); DCHECK(tab_contents_collector_map_.empty()); - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); NotificationService::current()->Notify(NOTIFY_TAB_CLOSED, Source<NavigationController>(this), @@ -221,7 +223,11 @@ TabContents* NavigationController::GetTabContents(TabContentsType t) { } void NavigationController::Reload() { - DiscardPendingEntryInternal(); + // Reloading a transient entry does nothing. + if (transient_entry_index_ != -1) + return; + + DiscardNonCommittedEntriesInternal(); int current_index = GetCurrentEntryIndex(); if (check_for_repost_ && current_index != -1 && GetEntryAtIndex(current_index)->has_post_data() && @@ -242,7 +248,7 @@ void NavigationController::Reload() { if (current_index == -1) return; - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); pending_entry_index_ = current_index; entries_[pending_entry_index_]->set_transition_type(PageTransition::RELOAD); @@ -260,7 +266,7 @@ void NavigationController::LoadEntry(NavigationEntry* entry) { // When navigating to a new page, we don't know for sure if we will actually // end up leaving the current page. The new page load could for example // result in a download or a 'no content' response (e.g., a mailto: URL). - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); pending_entry_ = entry; NotificationService::current()->Notify( NOTIFY_NAV_ENTRY_PENDING, @@ -270,13 +276,16 @@ void NavigationController::LoadEntry(NavigationEntry* entry) { } NavigationEntry* NavigationController::GetActiveEntry() const { - NavigationEntry* entry = pending_entry_; - if (!entry) - entry = GetLastCommittedEntry(); - return entry; + if (transient_entry_index_ != -1) + return entries_[transient_entry_index_].get(); + if (pending_entry_) + return pending_entry_; + return GetLastCommittedEntry(); } int NavigationController::GetCurrentEntryIndex() const { + if (transient_entry_index_ != -1) + return transient_entry_index_; if (pending_entry_index_ != -1) return pending_entry_index_; return last_committed_entry_index_; @@ -289,7 +298,9 @@ NavigationEntry* NavigationController::GetLastCommittedEntry() const { } NavigationEntry* NavigationController::GetEntryAtOffset(int offset) const { - int index = last_committed_entry_index_ + offset; + int index = (transient_entry_index_ != -1) ? + transient_entry_index_ + offset : + last_committed_entry_index_ + offset; if (index < 0 || index >= GetEntryCount()) return NULL; @@ -314,7 +325,7 @@ void NavigationController::GoBack() { // Base the navigation on where we are now... int current_index = GetCurrentEntryIndex(); - DiscardPendingEntry(); + DiscardNonCommittedEntries(); pending_entry_index_ = current_index - 1; NavigateToPendingEntry(false); @@ -326,12 +337,19 @@ void NavigationController::GoForward() { return; } + bool transient = (transient_entry_index_ != -1); + // Base the navigation on where we are now... int current_index = GetCurrentEntryIndex(); - DiscardPendingEntry(); + DiscardNonCommittedEntries(); + + pending_entry_index_ = current_index; + // If there was a transient entry, we removed it making the current index + // the next page. + if (!transient) + pending_entry_index_++; - pending_entry_index_ = current_index + 1; NavigateToPendingEntry(false); } @@ -341,14 +359,27 @@ void NavigationController::GoToIndex(int index) { return; } - DiscardPendingEntry(); + if (transient_entry_index_ != -1) { + if (index == transient_entry_index_) { + // Nothing to do when navigating to the transient. + return; + } + if (index > transient_entry_index_) { + // Removing the transient is goint to shift all entries by 1. + index--; + } + } + + DiscardNonCommittedEntries(); pending_entry_index_ = index; NavigateToPendingEntry(false); } void NavigationController::GoToOffset(int offset) { - int index = last_committed_entry_index_ + offset; + int index = (transient_entry_index_ != -1) ? + transient_entry_index_ + offset : + last_committed_entry_index_ + offset; if (index < 0 || index >= GetEntryCount()) return; @@ -359,6 +390,34 @@ void NavigationController::ReloadDontCheckForRepost() { Reload(); } +void NavigationController::RemoveEntryAtIndex(int index, + const GURL& default_url) { + int size = static_cast<int>(entries_.size()); + DCHECK(index < size); + + DiscardNonCommittedEntries(); + + entries_.erase(entries_.begin() + index); + + if (last_committed_entry_index_ == index) { + last_committed_entry_index_--; + // We removed the currently shown entry, so we have to load something else. + if (last_committed_entry_index_ != -1) { + pending_entry_index_ = last_committed_entry_index_; + NavigateToPendingEntry(false); + } else { + // If there is nothing to show, show a default page. + LoadURL(default_url.is_empty() ? GURL("about:blank") : default_url, + PageTransition::START_PAGE); + } + } else if (last_committed_entry_index_ > index) { + last_committed_entry_index_--; + } + + // TODO(brettw) bug 1324021: we probably need some notification here so the + // session service can stay in sync. +} + void NavigationController::Destroy() { // Close all tab contents owned by this controller. We make a list on the // stack because they are removed from the map as they are Destroyed @@ -444,6 +503,18 @@ NavigationEntry* NavigationController::CreateNavigationEntry( return entry; } +void NavigationController::AddTransientEntry(NavigationEntry* entry) { + // Discard any current transient entry, we can only have one at a time. + int index = 0; + if (last_committed_entry_index_ != -1) + index = last_committed_entry_index_ + 1; + DiscardTransientEntry(); + entries_.insert(entries_.begin() + index, linked_ptr<NavigationEntry>(entry)); + transient_entry_index_ = index; + active_contents_->NotifyNavigationStateChanged( + TabContents::INVALIDATE_EVERYTHING); +} + void NavigationController::LoadURL(const GURL& url, PageTransition::Type transition) { // The user initiated a load, we don't need to reload anymore. @@ -463,7 +534,7 @@ void NavigationController::LoadURLLazily(const GURL& url, if (icon) entry->favicon().set_bitmap(*icon); - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); pending_entry_ = entry; load_pending_entry_when_active_ = true; } @@ -710,7 +781,7 @@ void NavigationController::RendererDidNavigateToExistingPage( // Note that we need to use the "internal" version since we don't want to // actually change any other state, just kill the pointer. if (entry == pending_entry_) - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); last_committed_entry_index_ = entry_index; } @@ -730,7 +801,7 @@ void NavigationController::RendererDidNavigateToSamePage( // a regular user-initiated navigation. existing_entry->set_unique_id(pending_entry_->unique_id()); - DiscardPendingEntry(); + DiscardNonCommittedEntries(); } void NavigationController::RendererDidNavigateInPage( @@ -793,6 +864,8 @@ bool NavigationController::RendererDidNavigateAutoSubframe( } void NavigationController::CommitPendingEntry() { + DiscardTransientEntry(); + if (!GetPendingEntry()) return; // Nothing to do. @@ -810,7 +883,7 @@ void NavigationController::CommitPendingEntry() { // committing. Just mark it as committed. details.type = NavigationType::EXISTING_PAGE; int new_entry_index = pending_entry_index_; - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); // Mark that entry as committed. last_committed_entry_index_ = new_entry_index; @@ -845,56 +918,6 @@ int NavigationController::GetIndexOfEntry( return (i == entries_.end()) ? -1 : static_cast<int>(i - entries_.begin()); } -void NavigationController::RemoveLastEntryForInterstitial() { - int current_size = static_cast<int>(entries_.size()); - - if (current_size > 0) { - if (pending_entry_ == entries_[current_size - 1] || - pending_entry_index_ == current_size - 1) - DiscardPendingEntryInternal(); - - entries_.pop_back(); - - if (last_committed_entry_index_ >= current_size - 1) { - last_committed_entry_index_ = current_size - 2; - - if (last_committed_entry_index_ != -1) { - // Broadcast the notification of the navigation. This is kind of a hack, - // since the navigation wasn't actually committed. But this function is - // used for interstital pages, and the UI needs to get updated when the - // interstitial page - LoadCommittedDetails details; - details.entry = GetActiveEntry(); - details.is_auto = false; - details.is_in_page = false; - details.is_main_frame = true; - NotifyNavigationEntryCommitted(&details); - } - } - - NotifyPrunedEntries(this, false, 1); - } -} - -void NavigationController::AddDummyEntryForInterstitial( - const NavigationEntry& clone_me) { - // We need to send a commit notification for this transition. - LoadCommittedDetails details; - if (GetLastCommittedEntry()) - details.previous_url = GetLastCommittedEntry()->url(); - - NavigationEntry* new_entry = new NavigationEntry(clone_me); - InsertEntry(new_entry); - // Watch out, don't use clone_me after this. The caller may have passed in a - // reference to our pending entry, which means it would have been destroyed. - - details.is_auto = false; - details.entry = GetActiveEntry(); - details.is_in_page = false; - details.is_main_frame = true; - NotifyNavigationEntryCommitted(&details); -} - bool NavigationController::IsURLInPageNavigation(const GURL& url) const { NavigationEntry* last_committed = GetLastCommittedEntry(); if (!last_committed) @@ -902,8 +925,9 @@ bool NavigationController::IsURLInPageNavigation(const GURL& url) const { return AreURLsInPageNavigation(last_committed->url(), url); } -void NavigationController::DiscardPendingEntry() { - DiscardPendingEntryInternal(); +void NavigationController::DiscardNonCommittedEntries() { + bool transient = transient_entry_index_ != -1; + DiscardNonCommittedEntriesInternal(); // Synchronize the active_contents_ to the last committed entry. NavigationEntry* last_entry = GetLastCommittedEntry(); @@ -933,6 +957,13 @@ void NavigationController::DiscardPendingEntry() { DCHECK(from_contents != active_contents_); ScheduleTabContentsCollection(from_contents->type()); } + + // If there was a transient entry, invalidate everything so the new active + // entry state is shown. + if (transient) { + active_contents_->NotifyNavigationStateChanged( + TabContents::INVALIDATE_EVERYTHING); + } } void NavigationController::InsertEntry(NavigationEntry* entry) { @@ -945,7 +976,7 @@ void NavigationController::InsertEntry(NavigationEntry* entry) { if (pending_entry) entry->set_unique_id(pending_entry->unique_id()); - DiscardPendingEntryInternal(); + DiscardNonCommittedEntriesInternal(); int current_size = static_cast<int>(entries_.size()); @@ -962,7 +993,7 @@ void NavigationController::InsertEntry(NavigationEntry* entry) { } if (entries_.size() >= max_entry_count_) { - RemoveEntryAtIndex(0); + RemoveEntryAtIndex(0, GURL()); NotifyPrunedEntries(this, true, 1); } @@ -1011,7 +1042,7 @@ void NavigationController::NavigateToPendingEntry(bool reload) { NavigationEntry temp_entry(*pending_entry_); if (!contents->NavigateToPendingEntry(reload)) - DiscardPendingEntry(); + DiscardNonCommittedEntries(); } void NavigationController::NotifyNavigationEntryCommitted( @@ -1115,28 +1146,6 @@ void NavigationController::NotifyEntryChanged(const NavigationEntry* entry, Details<EntryChangedDetails>(&det)); } -void NavigationController::RemoveEntryAtIndex(int index) { - // TODO(brettw) this is only called to remove the first one when we've got - // too many entries. It should probably be more specific for this case. - if (index >= static_cast<int>(entries_.size()) || - index == pending_entry_index_ || index == last_committed_entry_index_) { - NOTREACHED(); - return; - } - - entries_.erase(entries_.begin() + index); - - if (last_committed_entry_index_ >= index) { - if (!entries_.empty()) - last_committed_entry_index_--; - else - last_committed_entry_index_ = -1; - } - - // TODO(brettw) bug 1324021: we probably need some notification here so the - // session service can stay in sync. -} - NavigationController* NavigationController::Clone(HWND parent_hwnd) { NavigationController* nc = new NavigationController(NULL, profile_); @@ -1210,11 +1219,20 @@ void NavigationController::FinishRestore(HWND parent_hwnd, int selected_index) { GetTabContentsCreateIfNecessary(parent_hwnd, *entries_[selected_index]); } -void NavigationController::DiscardPendingEntryInternal() { +void NavigationController::DiscardNonCommittedEntriesInternal() { if (pending_entry_index_ == -1) delete pending_entry_; pending_entry_ = NULL; pending_entry_index_ = -1; + + DiscardTransientEntry(); +} + +void NavigationController::DiscardTransientEntry() { + if (transient_entry_index_ == -1) + return; + entries_.erase(entries_.begin() + transient_entry_index_ ); + transient_entry_index_ = -1; } int NavigationController::GetEntryIndexWithPageID( @@ -1227,3 +1245,9 @@ int NavigationController::GetEntryIndexWithPageID( } return -1; } + +NavigationEntry* NavigationController::GetTransientEntry() const { + if (transient_entry_index_ == -1) + return NULL; + return entries_[transient_entry_index_].get(); +} diff --git a/chrome/browser/navigation_controller.h b/chrome/browser/navigation_controller.h index 1cb39aa..cffee64 100644 --- a/chrome/browser/navigation_controller.h +++ b/chrome/browser/navigation_controller.h @@ -151,10 +151,11 @@ class NavigationController { // Active entry -------------------------------------------------------------- - // Returns the active entry, which is the pending entry if a navigation is in - // progress or the last committed entry otherwise. NOTE: This can be NULL!! + // Returns the active entry, which is the transient entry if any, the pending + // entry if a navigation is in progress or the last committed entry otherwise. + // NOTE: This can be NULL!! // - // If you are trying to get the current state of the NavigationControllerBase, + // If you are trying to get the current state of the NavigationController, // this is the method you will typically want to call. // NavigationEntry* GetActiveEntry() const; @@ -175,8 +176,9 @@ class NavigationController { // Navigation list ----------------------------------------------------------- - // Returns the number of entries in the NavigationControllerBase, excluding - // the pending entry if there is one. + // Returns the number of entries in the NavigationController, excluding + // the pending entry if there is one, but including the transient entry if + // any. int GetEntryCount() const { return static_cast<int>(entries_.size()); } @@ -190,7 +192,7 @@ class NavigationController { NavigationEntry* GetEntryAtOffset(int offset) const; // Returns the index of the specified entry, or -1 if entry is not contained - // in this NavigationControllerBase. + // in this NavigationController. int GetIndexOfEntry(const NavigationEntry* entry) const; // Return the index of the entry with the corresponding type, instance, and @@ -219,9 +221,10 @@ class NavigationController { // new page ID for you and update the TabContents with that ID. void CommitPendingEntry(); - // Calling this may cause the active tab contents to switch if the current - // entry corresponds to a different tab contents type. - void DiscardPendingEntry(); + // Discards the pending and transient entries if any. Calling this may cause + // the active tab contents to switch if the current entry corresponds to a + // different tab contents type. + void DiscardNonCommittedEntries(); // Returns the pending entry corresponding to the navigation that is // currently in progress, or null if there is none. @@ -235,6 +238,21 @@ class NavigationController { return pending_entry_index_; } + // Transient entry ----------------------------------------------------------- + + // Adds an entry that is returned by GetActiveEntry(). The entry is + // transient: any navigation causes it to be removed and discarded. + // The NavigationController becomes the owner of |entry| and deletes it when + // it discards it. This is useful with interstitial page that need to be + // represented as an entry, but should go away when the user navigates away + // from them. + // Note that adding a transient entry does not change the active contents. + void AddTransientEntry(NavigationEntry* entry); + + // Returns the transient entry if any. Note that the returned entry is owned + // by the navigation controller and may be deleted at any time. + NavigationEntry* GetTransientEntry() const; + // New navigations ----------------------------------------------------------- // Loads the specified URL. @@ -270,6 +288,14 @@ class NavigationController { // Same as Reload, but doesn't check if current entry has POST data. void ReloadDontCheckForRepost(); + // Removing of entries ------------------------------------------------------- + + // Removes the entry at the specified |index|. This call dicards any pending + // and transient entries. |default_url| is the URL that the navigation + // controller navigates to if there are no more entries after the removal. + // If |default_url| is empty, we default to "about:blank". + void RemoveEntryAtIndex(int index, const GURL& default_url); + // TabContents --------------------------------------------------------------- // Notifies the controller that a TabContents that it owns has been destroyed. @@ -303,17 +329,6 @@ class NavigationController { bool is_interstitial, LoadCommittedDetails* details); - // Inserts a new entry by making a copy of the given navigation entry. This is - // used by interstitials to create dummy entries that they will be in charge - // of removing later. - void AddDummyEntryForInterstitial(const NavigationEntry& clone_me); - - // Removes the last entry in the list. This is used by the interstitial code - // to delete the dummy entry created by AddDummyEntryForInterstitial. If the - // last entry is the currently committed one, a ENTRY_COMMITTED notification - // will be broadcast. - void RemoveLastEntryForInterstitial(); - // Notifies us that we just became active. This is used by the TabContents // so that we know to load URLs that were pending as "lazy" loads. void SetActive(bool is_active); @@ -426,10 +441,6 @@ class NavigationController { // and deleted by this navigation controller void RegisterTabContents(TabContents* some_contents); - // Removes the entry at the specified index. Note that you should not remove - // the pending entry or the last committed entry. - void RemoveEntryAtIndex(int index); - // Sets the max restored page ID this NavigationController has seen, if it // was restored from a previous session. void set_max_restored_page_id(int max_id) { max_restored_page_id_ = max_id; } @@ -458,8 +469,12 @@ class NavigationController { // The new entry will become the active one. void InsertEntry(NavigationEntry* entry); - // Discards the pending entry without updating active_contents_ - void DiscardPendingEntryInternal(); + // Discards the pending and transient entries without updating + // active_contents_. + void DiscardNonCommittedEntriesInternal(); + + // Discards the transient entry without updating active_contents_. + void DiscardTransientEntry(); // --------------------------------------------------------------------------- @@ -486,6 +501,13 @@ class NavigationController { // new entry (created by LoadURL). int pending_entry_index_; + // The index for the entry that is shown until a navigation occurs. This is + // used for interstitial pages. -1 if there are no such entry. + // Note that this entry really appears in the list of entries, but only + // temporarily (until the next navigation). Any index poiting to an entry + // after the transient entry will become invalid if you navigate forward. + int transient_entry_index_; + // Tab contents. One entry per type used. The tab controller owns // every tab contents used. typedef std::map<TabContentsType, TabContents*> TabContentsMap; diff --git a/chrome/browser/navigation_controller_unittest.cc b/chrome/browser/navigation_controller_unittest.cc index 759ad6f..ed27d84 100644 --- a/chrome/browser/navigation_controller_unittest.cc +++ b/chrome/browser/navigation_controller_unittest.cc @@ -429,7 +429,7 @@ TEST_F(NavigationControllerTest, LoadURL_Discarded) { EXPECT_TRUE(notifications.Check1AndReset(NOTIFY_NAV_ENTRY_COMMITTED)); contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->controller()->DiscardPendingEntry(); + contents->controller()->DiscardNonCommittedEntries(); EXPECT_EQ(0, notifications.size()); // Should not have produced a new session history entry. @@ -1140,7 +1140,7 @@ TEST_F(NavigationControllerTest, SwitchTypes_Discard) { // The tab contents should have been replaced ASSERT_TRUE(initial_contents != contents); - contents->controller()->DiscardPendingEntry(); + contents->controller()->DiscardNonCommittedEntries(); EXPECT_EQ(0, notifications.size()); // The tab contents should have been replaced back @@ -1352,6 +1352,186 @@ TEST_F(NavigationControllerTest, Interstitial) { contents->controller()->GetLastCommittedEntry()->page_type()); } +TEST_F(NavigationControllerTest, RemoveEntry) { + const GURL url1("test1:foo1"); + const GURL url2("test1:foo2"); + const GURL url3("test1:foo3"); + const GURL url4("test1:foo4"); + const GURL url5("test1:foo5"); + const GURL pending_url("test1:pending"); + const GURL default_url("test1:default"); + + contents->controller()->LoadURL(url1, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(0, url1); + contents->controller()->LoadURL(url2, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(1, url2); + contents->controller()->LoadURL(url3, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(2, url3); + contents->controller()->LoadURL(url4, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(3, url4); + contents->controller()->LoadURL(url5, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(4, url5); + + // Remove the last entry. + contents->controller()->RemoveEntryAtIndex( + contents->controller()->GetEntryCount() - 1, default_url); + EXPECT_EQ(4, contents->controller()->GetEntryCount()); + EXPECT_EQ(3, contents->controller()->GetLastCommittedEntryIndex()); + NavigationEntry* pending_entry = contents->controller()->GetPendingEntry(); + EXPECT_TRUE(pending_entry && pending_entry->url() == url4); + + // Add a pending entry. + contents->controller()->LoadURL(pending_url, PageTransition::TYPED); + // Now remove the last entry. + contents->controller()->RemoveEntryAtIndex( + contents->controller()->GetEntryCount() - 1, default_url); + // The pending entry should have been discarded and the last committed entry + // removed. + EXPECT_EQ(3, contents->controller()->GetEntryCount()); + EXPECT_EQ(2, contents->controller()->GetLastCommittedEntryIndex()); + pending_entry = contents->controller()->GetPendingEntry(); + EXPECT_TRUE(pending_entry && pending_entry->url() == url3); + + // Remove an entry which is not the last committed one. + contents->controller()->RemoveEntryAtIndex(0, default_url); + EXPECT_EQ(2, contents->controller()->GetEntryCount()); + EXPECT_EQ(1, contents->controller()->GetLastCommittedEntryIndex()); + // No navigation should have been initiated since we did not remove the + // current entry. + EXPECT_FALSE(contents->controller()->GetPendingEntry()); + + // Remove the 2 remaining entries. + contents->controller()->RemoveEntryAtIndex(1, default_url); + contents->controller()->RemoveEntryAtIndex(0, default_url); + + // This should have created a pending default entry. + EXPECT_EQ(0, contents->controller()->GetEntryCount()); + EXPECT_EQ(-1, contents->controller()->GetLastCommittedEntryIndex()); + pending_entry = contents->controller()->GetPendingEntry(); + EXPECT_TRUE(pending_entry && pending_entry->url() == default_url); +} + +// Tests the transient entry, making sure it goes away with all navigations. +TEST_F(NavigationControllerTest, TransientEntry) { + TestNotificationTracker notifications; + RegisterForAllNavNotifications(¬ifications, contents->controller()); + + const GURL url0("test1:foo0"); + const GURL url1("test1:foo1"); + const GURL url2("test1:foo2"); + const GURL url3("test1:foo3"); + const GURL url4("test1:foo4"); + const GURL transient_url("test1:transient"); + + contents->controller()->LoadURL(url0, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(0, url0); + contents->controller()->LoadURL(url1, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(1, url1); + + notifications.Reset(); + + // Adding a transient with no pending entry. + NavigationEntry* transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + + // We should not have received any notifications. + EXPECT_EQ(0, notifications.size()); + + // Check our state. + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + EXPECT_EQ(contents->controller()->GetEntryCount(), 3); + EXPECT_EQ(contents->controller()->GetLastCommittedEntryIndex(), 1); + EXPECT_EQ(contents->controller()->GetPendingEntryIndex(), -1); + EXPECT_TRUE(contents->controller()->GetLastCommittedEntry()); + EXPECT_FALSE(contents->controller()->GetPendingEntry()); + EXPECT_TRUE(contents->controller()->CanGoBack()); + EXPECT_FALSE(contents->controller()->CanGoForward()); + EXPECT_EQ(contents->GetMaxPageID(), 1); + + // Navigate. + contents->controller()->LoadURL(url2, PageTransition::TYPED); + contents->CompleteNavigationAsRenderer(2, url2); + + // We should have navigated, transient entry should be gone. + EXPECT_EQ(url2, contents->controller()->GetActiveEntry()->url()); + EXPECT_EQ(contents->controller()->GetEntryCount(), 3); + + // Add a transient again, then navigate with no pending entry this time. + transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + contents->CompleteNavigationAsRenderer(3, url3); + // Transient entry should be gone. + EXPECT_EQ(url3, contents->controller()->GetActiveEntry()->url()); + EXPECT_EQ(contents->controller()->GetEntryCount(), 4); + + // Initiate a navigation, add a transient then commit navigation. + contents->controller()->LoadURL(url4, PageTransition::TYPED); + transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + contents->CompleteNavigationAsRenderer(4, url4); + EXPECT_EQ(url4, contents->controller()->GetActiveEntry()->url()); + EXPECT_EQ(contents->controller()->GetEntryCount(), 5); + + // Add a transient and go back. This should simply remove the transient. + transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + EXPECT_TRUE(contents->controller()->CanGoBack()); + EXPECT_FALSE(contents->controller()->CanGoForward()); + contents->controller()->GoBack(); + // Transient entry should be gone. + EXPECT_EQ(url4, contents->controller()->GetActiveEntry()->url()); + EXPECT_EQ(contents->controller()->GetEntryCount(), 5); + contents->CompleteNavigationAsRenderer(3, url3); + + // Add a transient and go to an entry before the current one. + transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + contents->controller()->GoToIndex(1); + // The navigation should have been initiated, transient entry should be gone. + EXPECT_EQ(url1, contents->controller()->GetActiveEntry()->url()); + contents->CompleteNavigationAsRenderer(1, url1); + + // Add a transient and go to an entry after the current one. + transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + contents->controller()->GoToIndex(3); + // The navigation should have been initiated, transient entry should be gone. + // Because of the transient entry that is removed, going to index 3 makes us + // land on url2. + EXPECT_EQ(url2, contents->controller()->GetActiveEntry()->url()); + contents->CompleteNavigationAsRenderer(2, url2); + + // Add a transient and go forward. + transient_entry = new NavigationEntry(TAB_CONTENTS_WEB); + transient_entry->set_url(transient_url); + contents->controller()->AddTransientEntry(transient_entry); + EXPECT_EQ(transient_url, contents->controller()->GetActiveEntry()->url()); + EXPECT_TRUE(contents->controller()->CanGoForward()); + contents->controller()->GoForward(); + // We should have navigated, transient entry should be gone. + EXPECT_EQ(url3, contents->controller()->GetActiveEntry()->url()); + contents->CompleteNavigationAsRenderer(3, url3); + + // Ensure the URLS are correct. + EXPECT_EQ(contents->controller()->GetEntryCount(), 5); + EXPECT_EQ(contents->controller()->GetEntryAtIndex(0)->url(), url0); + EXPECT_EQ(contents->controller()->GetEntryAtIndex(1)->url(), url1); + EXPECT_EQ(contents->controller()->GetEntryAtIndex(2)->url(), url2); + EXPECT_EQ(contents->controller()->GetEntryAtIndex(3)->url(), url3); + EXPECT_EQ(contents->controller()->GetEntryAtIndex(4)->url(), url4); +} + // Tests that IsInPageNavigation returns appropriate results. Prevents // regression for bug 1126349. TEST_F(NavigationControllerTest, IsInPageNavigation) { diff --git a/chrome/browser/render_view_host_manager.cc b/chrome/browser/render_view_host_manager.cc index 7a57d8c..96a4653 100644 --- a/chrome/browser/render_view_host_manager.cc +++ b/chrome/browser/render_view_host_manager.cc @@ -6,7 +6,7 @@ #include "base/command_line.h" #include "base/logging.h" -#include "chrome/browser/interstitial_page_delegate.h" +#include "chrome/browser/interstitial_page.h" #include "chrome/browser/navigation_controller.h" #include "chrome/browser/navigation_entry.h" #include "chrome/browser/render_widget_host_view.h" @@ -37,7 +37,7 @@ RenderViewHostManager::RenderViewHostManager( original_render_view_host_(NULL), interstitial_render_view_host_(NULL), pending_render_view_host_(NULL), - interstitial_delegate_(NULL), + interstitial_page_(NULL), showing_repost_interstitial_(false) { } @@ -381,8 +381,7 @@ void RenderViewHostManager::ShouldClosePage(bool proceed) { } void RenderViewHostManager::ShowInterstitialPage( - const std::string& html_text, - InterstitialPageDelegate* delegate) { + InterstitialPage* interstitial_page) { // Note that it is important that the interstitial page render view host is // in the same process as the normal render view host for the tab, so they // use page ids from the same pool. If they came from different processes, @@ -477,7 +476,7 @@ void RenderViewHostManager::ShowInterstitialPage( // Create a pending renderer and move to ENTERING_INTERSTITIAL. interstitial_render_view_host_ = CreateRenderViewHost(interstitial_instance, MSG_ROUTING_NONE, NULL); - interstitial_delegate_ = delegate; + interstitial_page_ = interstitial_page; bool success = delegate_->CreateRenderViewForRenderManager( interstitial_render_view_host_); if (!success) { @@ -495,9 +494,10 @@ void RenderViewHostManager::ShowInterstitialPage( // We allow the DOM bindings as a way to get the page to talk back to us. interstitial_render_view_host_->AllowDomAutomationBindings(); - interstitial_render_view_host_->LoadAlternateHTMLString(html_text, false, - GURL::EmptyGURL(), - std::string()); + interstitial_render_view_host_->LoadAlternateHTMLString( + interstitial_page->GetHTMLContents(), false, + GURL::EmptyGURL(), + std::string()); } void RenderViewHostManager::HideInterstitialPage(bool wait_for_navigation, @@ -919,14 +919,9 @@ void RenderViewHostManager::DisableInterstitialProceed(bool stop_request) { void RenderViewHostManager::InterstitialPageGone() { DCHECK(!showing_interstitial_page()); - - NotificationService::current()->Notify( - NOTIFY_INTERSTITIAL_PAGE_CLOSED, - Source<NavigationController>(delegate_->GetControllerForRenderManager()), - NotificationService::NoDetails()); - if (interstitial_delegate_) { - interstitial_delegate_->InterstitialClosed(); - interstitial_delegate_ = NULL; + if (interstitial_page_) { + interstitial_page_->InterstitialClosed(); + interstitial_page_ = NULL; } } diff --git a/chrome/browser/render_view_host_manager.h b/chrome/browser/render_view_host_manager.h index 1be8d5c..45a5b69 100644 --- a/chrome/browser/render_view_host_manager.h +++ b/chrome/browser/render_view_host_manager.h @@ -12,7 +12,7 @@ #include "base/basictypes.h" #include "chrome/browser/render_view_host.h" -class InterstitialPageDelegate; +class InterstitialPage; class NavigationController; class NavigationEntry; class Profile; @@ -123,13 +123,12 @@ class RenderViewHostManager { // WebContents. void ShouldClosePage(bool proceed); - // Displays the specified html in the current page. This method can be used to - // show temporary pages (such as security error pages). It can be hidden by - // calling HideInterstitialPage, in which case the original page is restored. - // An optional delegate may be passed, it is owned by the caller and must - // remain valid while the interstitial does. - void ShowInterstitialPage(const std::string& html_text, - InterstitialPageDelegate* delegate); + // Displays an interstitial page in the current page. This method can be used + // to show temporary pages (such as security error pages). It can be hidden + // by calling HideInterstitialPage, in which case the original page is + // restored. The passed InterstitialPage is owned by the caller and must + // remain valid while the interstitial page is shown. + void ShowInterstitialPage(InterstitialPage* interstitial_page); // Reverts from the interstitial page to the original page. // If |wait_for_navigation| is true, the interstitial page is removed when @@ -166,13 +165,9 @@ class RenderViewHostManager { (renderer_state_ == LEAVING_INTERSTITIAL); } - // Accessors to the the interstitial delegate, that is optionaly set when - // an interstitial page is shown. - InterstitialPageDelegate* interstitial_delegate() const { - return interstitial_delegate_; - } - void set_interstitial_delegate(InterstitialPageDelegate* delegate) { - interstitial_delegate_ = delegate; + // Accessors to the the interstitial page. + InterstitialPage* interstitial_page() const { + return interstitial_page_; } private: @@ -258,11 +253,9 @@ class RenderViewHostManager { // immediately, because we are navigating away. void DisableInterstitialProceed(bool stop_request); - // Cleans up after an interstitial page is hidden, including removing the - // interstitial's NavigationEntry. + // Cleans up after an interstitial page is hidden. void InterstitialPageGone(); - // Our delegate, not owned by us. Guaranteed non-NULL. Delegate* delegate_; @@ -296,7 +289,9 @@ class RenderViewHostManager { // exist if an interstitial page is shown. RenderViewHost* pending_render_view_host_; - InterstitialPageDelegate* interstitial_delegate_; + // The intersitial page currently shown if any, not own by this class + // (the InterstitialPage is self-owned, it deletes itself when hidden). + InterstitialPage* interstitial_page_; // See comment above showing_repost_interstitial(). bool showing_repost_interstitial_; diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc index 7bae9c1..5aa292d 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc @@ -10,9 +10,11 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/browser_resources.h" #include "chrome/browser/dom_operation_notification_details.h" +#include "chrome/browser/dom_ui/new_tab_ui.h" #include "chrome/browser/google_util.h" #include "chrome/browser/navigation_controller.h" #include "chrome/browser/navigation_entry.h" +#include "chrome/browser/safe_browsing/safe_browsing_service.h" #include "chrome/browser/tab_util.h" #include "chrome/browser/web_contents.h" #include "chrome/common/jstemplate_builder.h" @@ -38,57 +40,27 @@ static const char* const kSbReportPhishingUrl = static const wchar_t* const kSbDiagnosticHtml = L"<a href=\"\" onClick=\"sendCommand(4); return false;\" onMouseDown=\"return false;\">%ls</a>"; -// Created on the io_thread. SafeBrowsingBlockingPage::SafeBrowsingBlockingPage( SafeBrowsingService* sb_service, - SafeBrowsingService::Client* client, - int render_process_host_id, - int render_view_id, - const GURL& url, - ResourceType::Type resource_type, - SafeBrowsingService::UrlCheckResult result) - : sb_service_(sb_service), - client_(client), - render_process_host_id_(render_process_host_id), - render_view_id_(render_view_id), - url_(url), - result_(result), + const SafeBrowsingService::BlockingPageParam& param) + : InterstitialPage(tab_util::GetTabContentsByID( + param.render_process_host_id, param.render_view_id), + param.resource_type == ResourceType::MAIN_FRAME, + param.url), + sb_service_(sb_service), + client_(param.client), + render_process_host_id_(param.render_process_host_id), + render_view_id_(param.render_view_id), + result_(param.result), proceed_(false), - tab_(NULL), - controller_(NULL), - delete_pending_(false), - is_main_frame_(resource_type == ResourceType::MAIN_FRAME), - created_temporary_entry_(false) { + did_notify_(false), + is_main_frame_(param.resource_type == ResourceType::MAIN_FRAME) { } -// Deleted on the io_thread. SafeBrowsingBlockingPage::~SafeBrowsingBlockingPage() { } -void SafeBrowsingBlockingPage::DisplayBlockingPage() { - TabContents* tab = tab_util::GetTabContentsByID(render_process_host_id_, - render_view_id_); - if (!tab || tab->type() != TAB_CONTENTS_WEB) { - NotifyDone(); - return; - } - - tab_ = tab; - controller_ = tab->controller(); - - // Register for notifications of events from this tab. - NotificationService* ns = NotificationService::current(); - DCHECK(ns); - ns->AddObserver(this, NOTIFY_TAB_CLOSING, - Source<NavigationController>(controller_)); - ns->AddObserver(this, NOTIFY_DOM_OPERATION_RESPONSE, - Source<TabContents>(tab_)); - - // Hold an extra reference to ourself until the interstitial is gone. - AddRef(); - - WebContents* web_contents = tab->AsWebContents(); - +std::string SafeBrowsingBlockingPage::GetHTMLContents() { // Load the HTML page and create the template components. DictionaryValue strings; ResourceBundle& rb = ResourceBundle::GetSharedInstance(); @@ -96,9 +68,10 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() { if (result_ == SafeBrowsingService::URL_MALWARE) { std::wstring link = StringPrintf(kSbDiagnosticHtml, - l10n_util::GetString(IDS_SAFE_BROWSING_MALWARE_DIAGNOSTIC_PAGE).c_str()); + l10n_util::GetString(IDS_SAFE_BROWSING_MALWARE_DIAGNOSTIC_PAGE). + c_str()); - strings.SetString(L"badURL", UTF8ToWide(url_.host())); + strings.SetString(L"badURL", UTF8ToWide(url().host())); strings.SetString(L"title", l10n_util::GetString(IDS_SAFE_BROWSING_MALWARE_TITLE)); strings.SetString(L"headLine", @@ -109,20 +82,20 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() { if (is_main_frame_) { strings.SetString(L"description1", l10n_util::GetStringF(IDS_SAFE_BROWSING_MALWARE_DESCRIPTION1, - UTF8ToWide(url_.host()))); + UTF8ToWide(url().host()))); strings.SetString(L"description2", l10n_util::GetStringF(IDS_SAFE_BROWSING_MALWARE_DESCRIPTION2, link, - UTF8ToWide(url_.host()))); + UTF8ToWide(url().host()))); } else { strings.SetString(L"description1", l10n_util::GetStringF(IDS_SAFE_BROWSING_MALWARE_DESCRIPTION4, - UTF8ToWide(tab_->GetURL().host()), - UTF8ToWide(url_.host()))); + UTF8ToWide(tab()->GetURL().host()), + UTF8ToWide(url().host()))); strings.SetString(L"description2", l10n_util::GetStringF(IDS_SAFE_BROWSING_MALWARE_DESCRIPTION5, link, - UTF8ToWide(url_.host()))); + UTF8ToWide(url().host()))); } strings.SetString(L"description3", @@ -137,17 +110,17 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() { (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) ? L"rtl" : L"ltr"); html = rb.GetDataResource(IDR_SAFE_BROWSING_MALWARE_BLOCK); - } else { + } else { // Phishing. strings.SetString(L"title", l10n_util::GetString(IDS_SAFE_BROWSING_PHISHING_TITLE)); strings.SetString(L"headLine", l10n_util::GetString(IDS_SAFE_BROWSING_PHISHING_HEADLINE)); strings.SetString(L"description1", l10n_util::GetStringF(IDS_SAFE_BROWSING_PHISHING_DESCRIPTION1, - UTF8ToWide(url_.host()))); + UTF8ToWide(url().host()))); strings.SetString(L"description2", l10n_util::GetStringF(IDS_SAFE_BROWSING_PHISHING_DESCRIPTION2, - UTF8ToWide(url_.host()))); + UTF8ToWide(url().host()))); strings.SetString(L"continue_button", l10n_util::GetString(IDS_SAFE_BROWSING_PHISHING_PROCEED_BUTTON)); @@ -161,90 +134,13 @@ void SafeBrowsingBlockingPage::DisplayBlockingPage() { html = rb.GetDataResource(IDR_SAFE_BROWSING_PHISHING_BLOCK); } - std::string html_page(jstemplate_builder::GetTemplateHtml(html, - &strings, - "template_root")); - - // If the malware is the actual main frame and we have no pending entry - // (typically the navigation was initiated by the page), we create a fake - // navigation entry (so the location bar shows the page's URL). - if (is_main_frame_ && tab_->controller()->GetPendingEntryIndex() == -1) { - NavigationEntry new_entry(TAB_CONTENTS_WEB); - new_entry.set_url(url_); - new_entry.set_page_type(NavigationEntry::INTERSTITIAL_PAGE); - tab_->controller()->AddDummyEntryForInterstitial(new_entry); - created_temporary_entry_ = true; - } - - // Show the interstitial page. - web_contents->ShowInterstitialPage(html_page, this); -} - -void SafeBrowsingBlockingPage::Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details) { - switch (type) { - case NOTIFY_TAB_CLOSING: - HandleClose(); - break; - case NOTIFY_DOM_OPERATION_RESPONSE: - Continue(Details<DomOperationNotificationDetails>(details)->json()); - break; - default: - NOTREACHED(); - } -} - -void SafeBrowsingBlockingPage::InterstitialClosed() { - HandleClose(); -} - -bool SafeBrowsingBlockingPage::GoBack() { - WebContents* web_contents = tab_->AsWebContents(); - NavigationEntry* prev_entry = - web_contents->controller()->GetEntryAtOffset(-1); - - if (!prev_entry) { - // Nothing to go to, default to about:blank. Navigating will cause the - // interstitial to hide which will trigger "this" to be deleted. - tab_->controller()->LoadURL(GURL("about:blank"), - PageTransition::AUTO_BOOKMARK); - } else if (prev_entry->tab_type() != TAB_CONTENTS_WEB || - prev_entry->restored() || - !is_main_frame_) { - // We do navigate back if any of these is true: - // - the page is not a WebContents, its TabContents might have to be - // recreated. - // - we have not yet visited that navigation entry (typically session - // restore), in which case the page is not already available. - // - the interstitial was triggered by a sub-resource. In that case we - // really need to navigate, just hiding the interstitial would show the - // page containing the bad resource, and we don't want that. - web_contents->controller()->GoBack(); - } else { - // Otherwise, the user was viewing a page and navigated to a URL that was - // interrupted by an interstitial. Thus, we can just hide the interstitial - // and show the page the user was on before. - web_contents->HideInterstitialPage(false, false); - } - - // WARNING: at this point we are now either deleted or pending deletion from - // the IO thread. - - // Remove the navigation entry for the malware page. Note that we always - // remove the entry even if we did not create it as it has been flagged as - // malware and we don't want the user navigating back to it. - web_contents->controller()->RemoveLastEntryForInterstitial(); - - return true; + return jstemplate_builder::GetTemplateHtml(html, &strings, "template_root"); } -void SafeBrowsingBlockingPage::Continue(const std::string& user_action) { - TabContents* tab = tab_util::GetTabContentsByID(render_process_host_id_, - render_view_id_); - DCHECK(tab); - WebContents* web = tab->AsWebContents(); - if (user_action == "2") { +void SafeBrowsingBlockingPage::CommandReceived(const std::string& command) { + DCHECK(tab()->type() == TAB_CONTENTS_WEB); + WebContents* web = tab()->AsWebContents(); + if (command == "2") { // User pressed "Learn more". GURL url; if (result_ == SafeBrowsingService::URL_MALWARE) { @@ -257,76 +153,75 @@ void SafeBrowsingBlockingPage::Continue(const std::string& user_action) { web->OpenURL(url, CURRENT_TAB, PageTransition::LINK); return; } - if (user_action == "3") { + if (command == "3") { // User pressed "Report error" for a phishing site. // Note that we cannot just put a link in the interstitial at this point. // It is not OK to navigate in the context of an interstitial page. DCHECK(result_ == SafeBrowsingService::URL_PHISHING); GURL report_url = safe_browsing_util::GeneratePhishingReportUrl(kSbReportPhishingUrl, - url_.spec()); + url().spec()); + web->HideInterstitialPage(false, false); web->OpenURL(report_url, CURRENT_TAB, PageTransition::LINK); return; } - if (user_action == "4") { + if (command == "4") { // We're going to take the user to Google's SafeBrowsing diagnostic page. std::string diagnostic = StringPrintf(kSbDiagnosticUrl, - EscapeQueryParamValue(url_.spec()).c_str()); + EscapeQueryParamValue(url().spec()).c_str()); GURL diagnostic_url(diagnostic); diagnostic_url = google_util::AppendGoogleLocaleParam(diagnostic_url); DCHECK(result_ == SafeBrowsingService::URL_MALWARE); + web->HideInterstitialPage(false, false); web->OpenURL(diagnostic_url, CURRENT_TAB, PageTransition::LINK); return; } - proceed_ = user_action == "1"; + proceed_ = command == "1"; if (proceed_) { - // We are continuing, if we have created a temporary navigation entry, - // delete it as a new will be created on navigation. - if (created_temporary_entry_) - web->controller()->RemoveLastEntryForInterstitial(); if (is_main_frame_) web->HideInterstitialPage(true, true); else web->HideInterstitialPage(false, false); } else { - GoBack(); + if (is_main_frame_) { + DontProceed(); + } else { + NavigationController* controller = web->controller(); + controller->RemoveEntryAtIndex(controller->GetLastCommittedEntryIndex(), + NewTabUIURL()); + } } - NotifyDone(); } -void SafeBrowsingBlockingPage::HandleClose() { - NotificationService* ns = NotificationService::current(); - DCHECK(ns); - ns->RemoveObserver(this, NOTIFY_TAB_CLOSING, - Source<NavigationController>(controller_)); - ns->RemoveObserver(this, NOTIFY_DOM_OPERATION_RESPONSE, - Source<TabContents>(tab_)); - +void SafeBrowsingBlockingPage::InterstitialClosed() { NotifyDone(); - Release(); + InterstitialPage::InterstitialClosed(); + // We are now deleted. } void SafeBrowsingBlockingPage::NotifyDone() { - if (delete_pending_) + if (did_notify_) return; - delete_pending_ = true; - - if (tab_ && tab_->AsWebContents()) { - // Ensure the WebContents does not keep a pointer to us. - tab_->AsWebContents()->set_interstitial_delegate(NULL); - } + did_notify_ = true; base::Thread* io_thread = g_browser_process->io_thread(); if (!io_thread) return; + SafeBrowsingService::BlockingPageParam param; + param.url = url(); + param.result = result_; + param.client = client_; + param.render_process_host_id = render_process_host_id_; + param.render_view_id = render_view_id_; + param.proceed = proceed_; io_thread->message_loop()->PostTask(FROM_HERE, NewRunnableMethod( sb_service_, &SafeBrowsingService::OnBlockingPageDone, - this, client_, proceed_)); + param)); } diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h index 405a3e3..056d9ae 100644 --- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.h +++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.h @@ -9,7 +9,7 @@ // page with some options (go back, continue) to give the user a chance to avoid // the harmful page. // -// The SafeBrowsingBlockingPage is created by the SafeBrowsingService on the IO +// The SafeBrowsingBlockingPage is created by the SafeBrowsingService on the UI // thread when we've determined that a page is malicious. The operation of the // blocking page occurs on the UI thread, where it waits for the user to make a // decision about what to do: either go back or continue on. @@ -22,53 +22,31 @@ #define CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_BLOCKING_PAGE_H_ #include "base/logging.h" -#include "chrome/browser/interstitial_page_delegate.h" +#include "chrome/browser/interstitial_page.h" #include "chrome/browser/safe_browsing/safe_browsing_service.h" -#include "chrome/common/notification_service.h" #include "googleurl/src/gurl.h" class MessageLoop; class TabContents; class NavigationController; -class SafeBrowsingBlockingPage - : public InterstitialPageDelegate, - public base::RefCountedThreadSafe<SafeBrowsingBlockingPage>, - public NotificationObserver { +class SafeBrowsingBlockingPage : public InterstitialPage { public: - // Created and destroyed on the IO thread, operates on the UI thread. SafeBrowsingBlockingPage(SafeBrowsingService* service, - SafeBrowsingService::Client* client, - int render_process_host_id, - int render_view_id, - const GURL& url, - ResourceType::Type resource_type, - SafeBrowsingService::UrlCheckResult result); - ~SafeBrowsingBlockingPage(); + const SafeBrowsingService::BlockingPageParam& param); + virtual ~SafeBrowsingBlockingPage(); - // Display the page to the user. This method runs on the UI thread. - void DisplayBlockingPage(); - - // NotificationObserver interface, runs on the UI thread. - virtual void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - - const GURL& url() { return url_; } - int render_process_host_id() { return render_process_host_id_; } - int render_view_id() { return render_view_id_; } - SafeBrowsingService::UrlCheckResult result() { return result_; } - - // InterstitialPageDelegate methods: + // InterstitialPage method: + virtual std::string GetHTMLContents(); virtual void InterstitialClosed(); - virtual bool GoBack(); - private: - // Handle user action for blocking page navigation choices. - void Continue(const std::string& user_action); + protected: + // InterstitialPage method: + virtual void CommandReceived(const std::string& command); - // Tell the SafeBrowsingService that the handling of the current page is done. - void HandleClose(); + private: + // Tells the SafeBrowsingService that the handling of the current page is + // done. void NotifyDone(); private: @@ -76,35 +54,30 @@ class SafeBrowsingBlockingPage SafeBrowsingService* sb_service_; SafeBrowsingService::Client* client_; MessageLoop* report_loop_; + SafeBrowsingService::UrlCheckResult result_; - // For determining which tab to block. + // For determining which tab to block (note that we need this even though we + // have access to the tab as when the interstitial is showing, retrieving the + // tab RPH and RV id would return the ones of the interstitial, not the ones + // for the page containing the malware). + // TODO(jcampan): when we refactor the interstitial to run as a separate view + // that does not interact with the WebContents as much, we can + // get rid of these. int render_process_host_id_; int render_view_id_; - GURL url_; - SafeBrowsingService::UrlCheckResult result_; - // Inform the SafeBrowsingService whether we are continuing with this page // load or going back to the previous page. bool proceed_; - // Stored for use in the notification service, and are only used for their - // pointer value, but not for calling methods on. This is done to allow us to - // unregister as observers after the tab has gone (is NULL). - TabContents* tab_; - NavigationController* controller_; - - // Used for cleaning up after ourself. - bool delete_pending_; + // Whether we have notify the SafeBrowsingService yet that a decision had been + // made whether to proceed or block the unsafe resource. + bool did_notify_; // Whether the flagged resource is the main page (or a sub-resource is false). bool is_main_frame_; - // Whether we have created a temporary navigation entry as part of showing - // the blocking page. - bool created_temporary_entry_; - - DISALLOW_EVIL_CONSTRUCTORS(SafeBrowsingBlockingPage); + DISALLOW_COPY_AND_ASSIGN(SafeBrowsingBlockingPage); }; #endif // CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_BLOCKING_PAGE_H_ diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc index 9a35ddd..4f9a157 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.cc +++ b/chrome/browser/safe_browsing/safe_browsing_service.cc @@ -190,12 +190,25 @@ void SafeBrowsingService::DisplayBlockingPage(const GURL& url, } } - SafeBrowsingBlockingPage* blocking_page = new SafeBrowsingBlockingPage( - this, client, render_process_host_id, render_view_id, url, resource_type, - result); - blocking_page->AddRef(); - ui_loop->PostTask(FROM_HERE, NewRunnableMethod( - blocking_page, &SafeBrowsingBlockingPage::DisplayBlockingPage)); + BlockingPageParam param; + param.url = url; + param.resource_type = resource_type; + param.result = result; + param.client = client; + param.render_process_host_id = render_process_host_id; + param.render_view_id = render_view_id; + // The blocking page must be created from the UI thread. + ui_loop->PostTask(FROM_HERE, NewRunnableMethod(this, + &SafeBrowsingService::DoDisplayBlockingPage, + param)); +} + +// Invoked on the UI thread. +void SafeBrowsingService::DoDisplayBlockingPage( + const BlockingPageParam& param) { + SafeBrowsingBlockingPage* blocking_page = new SafeBrowsingBlockingPage(this, + param); + blocking_page->Show(); } void SafeBrowsingService::CancelCheck(Client* client) { @@ -386,23 +399,19 @@ void SafeBrowsingService::DatabaseUpdateFinished() { GetDatabase()->UpdateFinished(); } -void SafeBrowsingService::OnBlockingPageDone(SafeBrowsingBlockingPage* page, - Client* client, - bool proceed) { - NotifyClientBlockingComplete(client, proceed); +void SafeBrowsingService::OnBlockingPageDone(const BlockingPageParam& param) { + NotifyClientBlockingComplete(param.client, param.proceed); - if (proceed) { + if (param.proceed) { // Whitelist this domain and warning type for the given tab. WhiteListedEntry entry; - entry.render_process_host_id = page->render_process_host_id(); - entry.render_view_id = page->render_view_id(); - entry.domain = net::RegistryControlledDomainService::GetDomainAndRegistry( - page->url()); - entry.result = page->result(); + entry.render_process_host_id = param.render_process_host_id; + entry.render_view_id = param.render_view_id; + entry.domain = + net::RegistryControlledDomainService::GetDomainAndRegistry(param.url); + entry.result = param.result; white_listed_entries_.push_back(entry); } - - page->Release(); } void SafeBrowsingService::NotifyClientBlockingComplete(Client* client, diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h index 1d25d53..aec465d 100644 --- a/chrome/browser/safe_browsing/safe_browsing_service.h +++ b/chrome/browser/safe_browsing/safe_browsing_service.h @@ -52,6 +52,18 @@ class SafeBrowsingService virtual void OnBlockingPageComplete(bool proceed) = 0; }; + // Structure used to pass parameters between the IO and UI thread when + // interacting with the blocking page. + struct BlockingPageParam { + GURL url; + bool proceed; + UrlCheckResult result; + Client* client; + int render_process_host_id; + int render_view_id; + ResourceType::Type resource_type; + }; + // Creates the safe browsing service. Need to initialize before using. SafeBrowsingService(); ~SafeBrowsingService(); @@ -125,9 +137,7 @@ class SafeBrowsingService void UpdateFinished(); // The blocking page on the UI thread has completed. - void OnBlockingPageDone(SafeBrowsingBlockingPage* page, - Client* client, - bool proceed); + void OnBlockingPageDone(const BlockingPageParam& param); // Called when the SafeBrowsingProtocolManager has received updated MAC keys. void OnNewMacKeys(const std::string& client_key, @@ -216,6 +226,9 @@ class SafeBrowsingService // power state. void HandleResume(); + // Invoked on the UI thread to show the blocking page. + void DoDisplayBlockingPage(const BlockingPageParam& param); + MessageLoop* io_loop_; typedef std::set<SafeBrowsingCheck*> CurrentChecks; diff --git a/chrome/browser/ssl_blocking_page.cc b/chrome/browser/ssl_blocking_page.cc index ba90efe..c92ddc2 100644 --- a/chrome/browser/ssl_blocking_page.cc +++ b/chrome/browser/ssl_blocking_page.cc @@ -11,8 +11,6 @@ #include "chrome/browser/dom_operation_notification_details.h" #include "chrome/browser/navigation_controller.h" #include "chrome/browser/navigation_entry.h" -#include "chrome/browser/profile.h" -#include "chrome/browser/render_view_host.h" #include "chrome/browser/ssl_error_info.h" #include "chrome/browser/tab_contents.h" #include "chrome/browser/web_contents.h" @@ -24,71 +22,17 @@ #include "generated_resources.h" -// static -SSLBlockingPage::SSLBlockingPageMap* - SSLBlockingPage::tab_to_blocking_page_ = NULL; - +// Note that we always create a navigation entry with SSL errors. +// No error happening loading a sub-resource triggers an interstitial so far. SSLBlockingPage::SSLBlockingPage(SSLManager::CertError* error, Delegate* delegate) - : error_(error), + : InterstitialPage(error->GetTabContents(), true, error->request_url()), + error_(error), delegate_(delegate), - delegate_has_been_notified_(false), - remove_last_entry_(true), - created_nav_entry_(false) { - InitSSLBlockingPageMap(); - // Remember the tab, because we might not be able to get to it later - // via the error. - tab_ = error->GetTabContents(); - DCHECK(tab_); - - // If there's already an interstitial in this tab, then we're about to - // replace it. We should be ok with just deleting the previous - // SSLBlockingPage (not hiding it first), since we're about to be shown. - SSLBlockingPageMap::const_iterator iter = tab_to_blocking_page_->find(tab_); - if (iter != tab_to_blocking_page_->end()) { - // Deleting the SSLBlockingPage will also remove it from the map. - delete iter->second; - - // Since WebContents::InterstitialPageGone won't be called, we need - // to clear the last NavigationEntry manually. - tab_->controller()->RemoveLastEntryForInterstitial(); - } - (*tab_to_blocking_page_)[tab_] = this; - - // Register notifications so we can delete ourself if the tab is closed. - NotificationService::current()->AddObserver(this, - NOTIFY_TAB_CLOSING, - Source<NavigationController>(tab_->controller())); - - NotificationService::current()->AddObserver(this, - NOTIFY_INTERSTITIAL_PAGE_CLOSED, - Source<NavigationController>(tab_->controller())); - - // Register for DOM operations, this is how the blocking page notifies us of - // what the user chooses. - NotificationService::current()->AddObserver(this, - NOTIFY_DOM_OPERATION_RESPONSE, - Source<TabContents>(tab_)); + delegate_has_been_notified_(false) { } SSLBlockingPage::~SSLBlockingPage() { - NotificationService::current()->RemoveObserver(this, - NOTIFY_TAB_CLOSING, - Source<NavigationController>(tab_->controller())); - - NotificationService::current()->RemoveObserver(this, - NOTIFY_INTERSTITIAL_PAGE_CLOSED, - Source<NavigationController>(tab_->controller())); - - NotificationService::current()->RemoveObserver(this, - NOTIFY_DOM_OPERATION_RESPONSE, - Source<TabContents>(tab_)); - - SSLBlockingPageMap::iterator iter = - tab_to_blocking_page_->find(tab_); - DCHECK(iter != tab_to_blocking_page_->end()); - tab_to_blocking_page_->erase(iter); - if (!delegate_has_been_notified_) { // The page is closed without the user having chosen what to do, default to // deny. @@ -96,7 +40,7 @@ SSLBlockingPage::~SSLBlockingPage() { } } -void SSLBlockingPage::Show() { +std::string SSLBlockingPage::GetHTMLContents() { // Let's build the html error page. DictionaryValue strings; SSLErrorInfo error_info = delegate_->GetSSLErrorInfo(error_); @@ -122,31 +66,15 @@ void SSLBlockingPage::Show() { ResourceBundle::GetSharedInstance().GetRawDataResource( IDR_SSL_ROAD_BLOCK_HTML)); - std::string html_text(jstemplate_builder::GetTemplateHtml(html, - &strings, - "template_root")); + return jstemplate_builder::GetTemplateHtml(html, &strings, "template_root"); +} - DCHECK(tab_->type() == TAB_CONTENTS_WEB); - WebContents* tab = tab_->AsWebContents(); +void SSLBlockingPage::UpdateEntry(NavigationEntry* entry) { + DCHECK(tab()->type() == TAB_CONTENTS_WEB); + WebContents* web = tab()->AsWebContents(); const net::SSLInfo& ssl_info = error_->ssl_info(); int cert_id = CertStore::GetSharedInstance()->StoreCert( - ssl_info.cert, tab->render_view_host()->process()->host_id()); - - if (tab_->controller()->GetPendingEntryIndex() == -1) { - // For new navigations, we just create a new navigation entry. - NavigationEntry new_entry(TAB_CONTENTS_WEB); - new_entry.set_url(error_->request_url()); - tab_->controller()->AddDummyEntryForInterstitial(new_entry); - created_nav_entry_ = true; - } else { - // When there is a pending entry index, that means we're doing a - // back/forward navigation. Clone that entry instead. - tab_->controller()->AddDummyEntryForInterstitial( - *tab_->controller()->GetPendingEntry()); - } - - NavigationEntry* entry = tab_->controller()->GetActiveEntry(); - entry->set_page_type(NavigationEntry::INTERSTITIAL_PAGE); + ssl_info.cert, web->render_view_host()->process()->host_id()); entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN); entry->ssl().set_cert_id(cert_id); @@ -154,101 +82,35 @@ void SSLBlockingPage::Show() { entry->ssl().set_security_bits(ssl_info.security_bits); NotificationService::current()->Notify( NOTIFY_SSL_STATE_CHANGED, - Source<NavigationController>(tab_->controller()), + Source<NavigationController>(web->controller()), NotificationService::NoDetails()); - - tab->ShowInterstitialPage(html_text, NULL); } -void SSLBlockingPage::Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details) { - switch (type) { - case NOTIFY_TAB_CLOSING: - case NOTIFY_INTERSTITIAL_PAGE_CLOSED: { - // We created a navigation entry for the interstitial, remove it. - // Note that we don't remove the entry if we are closing all tabs so that - // the last entry is kept for the restoring on next start-up. - Browser* browser = Browser::GetBrowserForController(tab_->controller(), - NULL); - // We may not have a browser (this is the case for constrained popups), in - // which case it does not matter if we do not remove the temporary entry - // as their navigation history is not saved. - if (remove_last_entry_ && browser && - !browser->tabstrip_model()->closing_all()) { - tab_->controller()->RemoveLastEntryForInterstitial(); - } - delete this; - break; - } - case NOTIFY_DOM_OPERATION_RESPONSE: { - std::string json = - Details<DomOperationNotificationDetails>(details)->json(); - if (json == "1") { - Proceed(); - } else { - DontProceed(); - } - break; - } - default: - NOTREACHED(); +void SSLBlockingPage::CommandReceived(const std::string& command) { + if (command == "1") { + Proceed(); + } else { + DontProceed(); } } void SSLBlockingPage::Proceed() { - // We hide the interstitial page first as allowing the certificate will - // resume the request and we want the WebContents back to showing the - // non interstitial page (otherwise the request completion messages may - // confuse the WebContents if it is still showing the interstitial - // page). - DCHECK(tab_->type() == TAB_CONTENTS_WEB); - tab_->AsWebContents()->HideInterstitialPage(true, true); + // We hide the interstitial page first (by calling Proceed()) as allowing the + // certificate will resume the request and we want the WebContents back to + // showing the non interstitial page (otherwise the request completion + // messages may confuse the WebContents if it is still showing the + // interstitial page). + InterstitialPage::Proceed(); // Accepting the certificate resumes the loading of the page. NotifyAllowCertificate(); - - // Do not remove the navigation entry if we have not created it explicitly - // as in such cases (session restore) the controller would not create a new - // entry on navigation since the page id is less than max page id. - if (!created_nav_entry_) - remove_last_entry_ = false; } void SSLBlockingPage::DontProceed() { NotifyDenyCertificate(); - - // We are navigating, remove the current entry before we mess with it. - remove_last_entry_ = false; - tab_->controller()->RemoveLastEntryForInterstitial(); - - NavigationEntry* entry = tab_->controller()->GetActiveEntry(); - if (!entry) { - // Nothing to go to, default to about:blank. Navigating will cause the - // interstitial to hide which will trigger "this" to be deleted. - tab_->controller()->LoadURL(GURL("about:blank"), - PageTransition::AUTO_BOOKMARK); - } else if (entry->tab_type() != TAB_CONTENTS_WEB) { - // Not a WebContent, reload it so to recreate the TabContents for it. - tab_->controller()->Reload(); - } else { - DCHECK(tab_->type() == TAB_CONTENTS_WEB); - if (entry->restored()) { - // If this page was restored, it is not available, we have to navigate to - // it. - tab_->controller()->GoToOffset(0); - } else { - tab_->AsWebContents()->HideInterstitialPage(false, false); - } - } - // WARNING: we are now deleted! + InterstitialPage::DontProceed(); } -// static -void SSLBlockingPage::InitSSLBlockingPageMap() { - if (!tab_to_blocking_page_) - tab_to_blocking_page_ = new SSLBlockingPageMap; -} void SSLBlockingPage::NotifyDenyCertificate() { DCHECK(!delegate_has_been_notified_); @@ -265,18 +127,6 @@ void SSLBlockingPage::NotifyAllowCertificate() { } // static -SSLBlockingPage* SSLBlockingPage::GetSSLBlockingPage( - TabContents* tab_contents) { - InitSSLBlockingPageMap(); - SSLBlockingPageMap::const_iterator iter = - tab_to_blocking_page_->find(tab_contents); - if (iter == tab_to_blocking_page_->end()) - return NULL; - - return iter->second; -} - -// static void SSLBlockingPage::SetExtraInfo( DictionaryValue* strings, const std::vector<std::wstring>& extra_info) { diff --git a/chrome/browser/ssl_blocking_page.h b/chrome/browser/ssl_blocking_page.h index 9663124..0b4e9c6 100644 --- a/chrome/browser/ssl_blocking_page.h +++ b/chrome/browser/ssl_blocking_page.h @@ -2,18 +2,19 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef CHROME_BROWSER_SSL_BLOCKING_PAGE_H__ -#define CHROME_BROWSER_SSL_BLOCKING_PAGE_H__ +#ifndef CHROME_BROWSER_SSL_BLOCKING_PAGE_H_ +#define CHROME_BROWSER_SSL_BLOCKING_PAGE_H_ #include <string> +#include "chrome/browser/interstitial_page.h" #include "chrome/browser/ssl_manager.h" #include "chrome/views/decision.h" // This class is responsible for showing/hiding the interstitial page that is // shown when a certificate error happens. // It deletes itself when the interstitial page is closed. -class SSLBlockingPage : public NotificationObserver { +class SSLBlockingPage : public InterstitialPage { public: // An interface that classes that want to interact with the SSLBlockingPage // should implement. @@ -30,45 +31,28 @@ class SSLBlockingPage : public NotificationObserver { virtual void OnAllowCertificate(SSLManager::CertError* error) = 0; }; - SSLBlockingPage(SSLManager::CertError* error, - Delegate* delegate); + SSLBlockingPage(SSLManager::CertError* error, Delegate* delegate); virtual ~SSLBlockingPage(); - void Show(); - - virtual void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - - // Invoked when the user clicks on proceed. - // Warning: 'this' has been deleted when this method returns. - void Proceed(); - - // Invoked when the user clicks on "take me out of here". - // Warning: 'this' has been deleted when this method returns. - void DontProceed(); - - // Retrieves the SSLBlockingPage if any associated with the specified - // |tab_contents| (used by ui tests). - static SSLBlockingPage* GetSSLBlockingPage(TabContents* tab_contents); - // A method that sets strings in the specified dictionary from the passed // vector so that they can be used to resource the ssl_roadblock.html/ // ssl_error.html files. // Note: there can be up to 5 strings in |extra_info|. static void SetExtraInfo(DictionaryValue* strings, const std::vector<std::wstring>& extra_info); - private: - typedef std::map<TabContents*,SSLBlockingPage*> SSLBlockingPageMap; - void NotifyDenyCertificate(); + protected: + // InterstitialPage implementation. + virtual std::string GetHTMLContents(); + virtual void CommandReceived(const std::string& command); + virtual void UpdateEntry(NavigationEntry* entry); + virtual void Proceed(); + virtual void DontProceed(); + private: + void NotifyDenyCertificate(); void NotifyAllowCertificate(); - // Initializes tab_to_blocking_page_ in a thread-safe manner. Should be - // called before accessing tab_to_blocking_page_. - static void InitSSLBlockingPageMap(); - // The error we represent. We will either call CancelRequest() or // ContinueRequest() on this object. scoped_refptr<SSLManager::CertError> error_; @@ -80,23 +64,8 @@ class SSLBlockingPage : public NotificationObserver { // A flag to indicate if we've notified |delegate_| of the user's decision. bool delegate_has_been_notified_; - // A flag used to know whether we should remove the last navigation entry from - // the navigation controller. - bool remove_last_entry_; - - // The tab in which we are displayed. - TabContents* tab_; - // Whether we created a fake navigation entry as part of showing the - // interstitial page. - bool created_nav_entry_; - - // We keep a map of the various blocking pages shown as the UI tests need to - // be able to retrieve them. - static SSLBlockingPageMap* tab_to_blocking_page_; - - DISALLOW_EVIL_CONSTRUCTORS(SSLBlockingPage); + DISALLOW_COPY_AND_ASSIGN(SSLBlockingPage); }; - -#endif // #ifndef CHROME_BROWSER_SSL_BLOCKING_PAGE_H__ +#endif // #ifndef CHROME_BROWSER_SSL_BLOCKING_PAGE_H_ diff --git a/chrome/browser/tab_contents.cc b/chrome/browser/tab_contents.cc index 44de4ab..2deb196 100644 --- a/chrome/browser/tab_contents.cc +++ b/chrome/browser/tab_contents.cc @@ -127,11 +127,17 @@ const GURL& TabContents::GetURL() const { } const std::wstring& TabContents::GetTitle() const { - // We always want to use the title for the last committed entry rather than - // a pending navigation entry. For example, when the user types in a URL, we - // want to keep the old page's title until the new load has committed and we - // get a new title. - NavigationEntry* entry = controller_->GetLastCommittedEntry(); + // We use the title for the last committed entry rather than a pending + // navigation entry. For example, when the user types in a URL, we want to + // keep the old page's title until the new load has committed and we get a new + // title. + // The exception is with transient pages, for which we really want to use + // their title, as they are not committed. + NavigationEntry* entry = controller_->GetTransientEntry(); + if (entry && !entry->title().empty()) + return entry->title(); + + entry = controller_->GetLastCommittedEntry(); if (entry) return entry->title(); else if (controller_->LoadingURLLazily()) @@ -166,7 +172,11 @@ const std::wstring TabContents::GetDefaultTitle() const { SkBitmap TabContents::GetFavIcon() const { // Like GetTitle(), we also want to use the favicon for the last committed // entry rather than a pending navigation entry. - NavigationEntry* entry = controller_->GetLastCommittedEntry(); + NavigationEntry* entry = controller_->GetTransientEntry(); + if (entry) + return entry->favicon().bitmap(); + + entry = controller_->GetLastCommittedEntry(); if (entry) return entry->favicon().bitmap(); else if (controller_->LoadingURLLazily()) diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc index 723aa4f..4f2d259 100644 --- a/chrome/browser/web_contents.cc +++ b/chrome/browser/web_contents.cc @@ -17,7 +17,7 @@ #include "chrome/browser/find_in_page_controller.h" #include "chrome/browser/find_notification_details.h" #include "chrome/browser/google_util.h" -#include "chrome/browser/interstitial_page_delegate.h" +#include "chrome/browser/interstitial_page.h" #include "chrome/browser/js_before_unload_handler.h" #include "chrome/browser/jsmessage_box_handler.h" #include "chrome/browser/load_from_memory_cache_details.h" @@ -1007,9 +1007,9 @@ void WebContents::UpdateTitle(RenderViewHost* rvh, (rvh == render_view_host())) { // We are showing an interstitial page in a different RenderViewHost, so // the page_id is not sufficient to find the entry from the controller. - // (both RenderViewHost page_ids overlap). We know it is the last entry, + // (both RenderViewHost page_ids overlap). We know it is the active entry, // so just use that. - entry = controller()->GetLastCommittedEntry(); + entry = controller()->GetActiveEntry(); } else { entry = controller()->GetEntryWithPageID(type(), GetSiteInstance(), page_id); @@ -1176,7 +1176,7 @@ void WebContents::DidFailProvisionalLoadWithError( // before the page loaded so that the discard would discard the wrong entry. NavigationEntry* pending_entry = controller()->GetPendingEntry(); if (pending_entry && pending_entry->url() == url) - controller()->DiscardPendingEntry(); + controller()->DiscardNonCommittedEntries(); render_manager_.RendererAbortedProvisionalLoad(render_view_host); } diff --git a/chrome/browser/web_contents.h b/chrome/browser/web_contents.h index 3fd2237..2b9bdee 100644 --- a/chrome/browser/web_contents.h +++ b/chrome/browser/web_contents.h @@ -145,22 +145,12 @@ class WebContents : public TabContents, return render_manager_.showing_repost_interstitial(); } - // The rest of the system wants to interact with the delegate our render view - // host manager has. See those setters for more. - InterstitialPageDelegate* interstitial_page_delegate() const { - return render_manager_.interstitial_delegate(); - } - void set_interstitial_delegate(InterstitialPageDelegate* delegate) { - render_manager_.set_interstitial_delegate(delegate); - } - - // Displays the specified html in the current page. This method can be used to - // show temporary pages (such as security error pages). It can be hidden by + // Displays the specified interstitial page. This method can be used to show + // temporary pages (such as security error pages). It can be hidden by // calling HideInterstitialPage, in which case the original page is restored. - // An optional delegate may be passed, it is not owned by the WebContents. - void ShowInterstitialPage(const std::string& html_text, - InterstitialPageDelegate* delegate) { - render_manager_.ShowInterstitialPage(html_text, delegate); + // |interstitial_page| is not owned by the WebContents. + void ShowInterstitialPage(InterstitialPage* interstitial_page) { + render_manager_.ShowInterstitialPage(interstitial_page); } // Reverts from the interstitial page to the original page. @@ -174,6 +164,11 @@ class WebContents : public TabContents, render_manager_.HideInterstitialPage(wait_for_navigation, proceed); } + // Returns the interstitial page currently shown if any, NULL otherwise. + InterstitialPage* interstitial_page() const { + return render_manager_.interstitial_page(); + } + // Misc state & callbacks ---------------------------------------------------- // More retarded pass-throughs (see also above under TabContents overrides). diff --git a/chrome/browser/web_contents_unittest.cc b/chrome/browser/web_contents_unittest.cc index 3620df8..932af42 100644 --- a/chrome/browser/web_contents_unittest.cc +++ b/chrome/browser/web_contents_unittest.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "base/logging.h" +#include "chrome/browser/interstitial_page.h" #include "chrome/browser/navigation_controller.h" #include "chrome/browser/navigation_entry.h" #include "chrome/browser/render_view_host.h" @@ -357,7 +358,11 @@ TEST_F(WebContentsTest, ShowInterstitialDontProceed) { EXPECT_TRUE(orig_rvh->is_loading); // Show interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); EXPECT_TRUE(contents->state_is_entering_interstitial()); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); EXPECT_TRUE(orig_rvh->is_loading); // Still loading in the background @@ -392,7 +397,11 @@ TEST_F(WebContentsTest, ShowInterstitialProceed) { contents->controller()->LoadURL(url, PageTransition::TYPED); // Show interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); // DidNavigate from the interstitial @@ -434,7 +443,11 @@ TEST_F(WebContentsTest, ShowInterstitialThenNavigate) { contents->controller()->LoadURL(url, PageTransition::TYPED); // Show interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); // DidNavigate from the interstitial @@ -481,7 +494,11 @@ TEST_F(WebContentsTest, ShowInterstitialIFrameNavigate) { // Show interstitial (in real world would probably be triggered by a resource // in the page). - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); EXPECT_TRUE(contents->state_is_entering_interstitial()); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); EXPECT_TRUE(interstitial_rvh->is_loading); @@ -518,7 +535,11 @@ TEST_F(WebContentsTest, VisitInterstitialURLTwice) { // Now navigate to an interstitial-inducing URL const GURL url2("https://www.google.com"); contents->controller()->LoadURL(url2, PageTransition::TYPED); - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); EXPECT_TRUE(contents->state_is_entering_interstitial()); int interstitial_delete_counter = 0; TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); @@ -537,7 +558,8 @@ TEST_F(WebContentsTest, VisitInterstitialURLTwice) { EXPECT_EQ(interstitial_rvh, contents->render_view_host()); // Interstitial shown a second time in a different RenderViewHost. - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + interstitial = new InterstitialPage(contents, true, interstitial_url); + interstitial->Show(); EXPECT_TRUE(contents->state_is_entering_interstitial()); // We expect the original interstitial has been deleted. EXPECT_EQ(interstitial_delete_counter, 1); @@ -704,7 +726,11 @@ TEST_F(WebContentsTest, CrossSiteInterstitialDontProceed) { TestRenderViewHost* pending_rvh = contents->pending_rvh(); // Show an interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); EXPECT_TRUE(contents->state_is_entering_interstitial()); EXPECT_EQ(orig_rvh, contents->render_view_host()); EXPECT_EQ(pending_rvh, contents->pending_rvh()); @@ -753,7 +779,11 @@ TEST_F(WebContentsTest, CrossSiteInterstitialProceed) { pending_rvh->set_delete_counter(&pending_rvh_delete_count); // Show an interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); // DidNavigate from the interstitial @@ -819,7 +849,11 @@ TEST_F(WebContentsTest, CrossSiteInterstitialThenNavigate) { contents->TestDidNavigate(orig_rvh, params1); // Show an interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + false, + interstitial_url); + interstitial->Show(); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); // DidNavigate from the interstitial @@ -875,7 +909,11 @@ TEST_F(WebContentsTest, CrossSiteInterstitialCrashThenNavigate) { pending_rvh->set_delete_counter(&pending_rvh_delete_count); // Show an interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); // DidNavigate from the interstitial @@ -938,7 +976,11 @@ TEST_F(WebContentsTest, CrossSiteInterstitialCrashesThenNavigate) { pending_rvh->set_delete_counter(&pending_rvh_delete_count); // Show an interstitial - contents->ShowInterstitialPage(std::string("Blocked"), NULL); + const GURL interstitial_url("http://interstitial"); + InterstitialPage* interstitial = new InterstitialPage(contents, + true, + interstitial_url); + interstitial->Show(); TestRenderViewHost* interstitial_rvh = contents->interstitial_rvh(); // DidNavigate from the interstitial |