diff options
| author | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-10-13 22:42:47 +0000 |
|---|---|---|
| committer | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2008-10-13 22:42:47 +0000 |
| commit | cbab76d1c74c93837bc76298d1a2e43646154194 (patch) | |
| tree | 7f1bdcd891e670b67eeac2993730c580698048eb /chrome | |
| parent | 3c1e4d080a8e69fb973638d1360d4d5dd0d2e4d5 (diff) | |
| download | chromium_src-cbab76d1c74c93837bc76298d1a2e43646154194.zip chromium_src-cbab76d1c74c93837bc76298d1a2e43646154194.tar.gz chromium_src-cbab76d1c74c93837bc76298d1a2e43646154194.tar.bz2 | |
This is the first pass at refactoring the interstitial page.
The SSL and malware blocking pages were doing similar things in 2 different classes.
There is now a base class called InterstitialPage that contains the common logic.
As part of that refactoring, the safe browsing was changed so that the SafeBrowsingBlockingPage is only used from the UI thread.
This CL also adds transient entries to the navigation controller: that type of entry gets deleted as soon as a navigation occurs. It is used by interstitial that need to create such a temporary entry while they show.
BUG=3013
TEST=Run the unit tests and ui tests.
Review URL: http://codereview.chromium.org/6311
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@3324 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
22 files changed, 892 insertions, 722 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 diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h index f6b1ad7..4340c9b 100644 --- a/chrome/common/notification_types.h +++ b/chrome/common/notification_types.h @@ -134,15 +134,6 @@ enum NotificationType { NOTIFY_DOWNLOAD_START, NOTIFY_DOWNLOAD_STOP, - // This is sent when an interstitial page showing in a WebContents is closed - // (as the result of a navigation to another page). The source is the - // WebContents the interstitial page is in. - // Note that you should not initiate a navigation as part of the processing of - // this notification, since this notification may be triggered as part of the - // destruction of the tab contents (the navigation controller would reuse - // the tab contents right before it would be destroyed). - NOTIFY_INTERSTITIAL_PAGE_CLOSED, - // Views --------------------------------------------------------------------- // Notification that a view was removed from a view hierarchy. The source is |