Pass the render view host with all provisional load related callbacks on the web contents observer (issue 10392072)

11 views
Skip to first unread message

joc...@chromium.org

unread,
May 13, 2012, 3:02:32 AM5/13/12
to a...@chromium.org, cr...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
Reviewers: Avi, creis,

Message:
Avi & Charlie, please review

Description:
Pass the render view host with all provisional load related callbacks on
the web
contents observer

There might be several provisional loads in different render view hosts
going
on, so the observers need to now where the signals came from

BUG=none
TEST=none, just added the parameter everywhere


Please review this at https://chromiumcodereview.appspot.com/10392072/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M chrome/browser/extensions/api/web_navigation/web_navigation_api.h
M chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
M chrome/browser/external_tab/external_tab_container_win.h
M chrome/browser/external_tab/external_tab_container_win.cc
M chrome/browser/instant/instant_loader.cc
M chrome/browser/prerender/prerender_tab_helper.h
M chrome/browser/prerender/prerender_tab_helper.cc
M chrome/browser/ssl/ssl_browser_tests.cc
M content/browser/browser_plugin/browser_plugin_web_contents_observer.h
M content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
M content/browser/web_contents/web_contents_impl.cc
M content/public/browser/web_contents_observer.h


Index: chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
diff --git
a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
index
69b41352747d41e7a4438bb7c47d39aaa1b11e4c..7bbd9cca72a0b8997a69c1ca0c51de3673f21060
100644
--- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
+++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
@@ -584,7 +584,8 @@ void
WebNavigationTabObserver::DidCommitProvisionalLoadForFrame(
int64 frame_id,
bool is_main_frame,
const GURL& url,
- content::PageTransition transition_type) {
+ content::PageTransition transition_type,
+ content::RenderViewHost* render_view_host) {
if (!navigation_state_.CanSendEvents(frame_id))
return;

@@ -624,7 +625,8 @@ void WebNavigationTabObserver::DidFailProvisionalLoad(
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) {
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) {
if (!navigation_state_.CanSendEvents(frame_id))
return;
navigation_state_.SetErrorOccurredInFrame(frame_id);
Index: chrome/browser/extensions/api/web_navigation/web_navigation_api.h
diff --git
a/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
b/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
index
6cdef16ca2274f5b7ad0c3e5c793864b4d172557..b5a3c964a5bb10af628b20322527dcd8d908420a
100644
--- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
+++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
@@ -156,13 +156,15 @@ class WebNavigationTabObserver : public
content::NotificationObserver,
int64 frame_id,
bool is_main_frame,
const GURL& url,
- content::PageTransition transition_type) OVERRIDE;
+ content::PageTransition transition_type,
+ content::RenderViewHost* render_view_host) OVERRIDE;
virtual void DidFailProvisionalLoad(
int64 frame_id,
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) OVERRIDE;
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) OVERRIDE;
virtual void DocumentLoadedInFrame(int64 frame_id) OVERRIDE;
virtual void DidFinishLoad(int64 frame_id,
const GURL& validated_url,
Index: chrome/browser/external_tab/external_tab_container_win.cc
diff --git a/chrome/browser/external_tab/external_tab_container_win.cc
b/chrome/browser/external_tab/external_tab_container_win.cc
index
d3ddaf72fbd851c546a43d635708f9352b908717..2f374bc5dd08d3c77549fb8897c3a343194a80c2
100644
--- a/chrome/browser/external_tab/external_tab_container_win.cc
+++ b/chrome/browser/external_tab/external_tab_container_win.cc
@@ -782,7 +782,8 @@ void ExternalTabContainer::DidFailProvisionalLoad(
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) {
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) {
automation_->Send(new AutomationMsg_NavigationFailed(
tab_handle_, error_code, validated_url));
ignore_next_load_notification_ = true;
Index: chrome/browser/external_tab/external_tab_container_win.h
diff --git a/chrome/browser/external_tab/external_tab_container_win.h
b/chrome/browser/external_tab/external_tab_container_win.h
index
523f2837ee33bb9c908310ed8389817b5492f4cc..8dc9b51ec8c0fdeee945c9470ad402ee1fa21618
100644
--- a/chrome/browser/external_tab/external_tab_container_win.h
+++ b/chrome/browser/external_tab/external_tab_container_win.h
@@ -202,7 +202,8 @@ class ExternalTabContainer : public
content::WebContentsDelegate,
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) OVERRIDE;
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) OVERRIDE;

// Message handlers
void OnForwardMessageToExternalHost(const std::string& message,
Index: chrome/browser/instant/instant_loader.cc
diff --git a/chrome/browser/instant/instant_loader.cc
b/chrome/browser/instant/instant_loader.cc
index
02ab3af7a150838435d01b3b9d726e7842d6dd4e..d8a15d6bcc53001447c3fdff030ebafb83cb9619
100644
--- a/chrome/browser/instant/instant_loader.cc
+++ b/chrome/browser/instant/instant_loader.cc
@@ -261,7 +261,8 @@ class InstantLoader::TabContentsDelegateImpl
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) OVERRIDE;
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) OVERRIDE;

private:
typedef std::vector<scoped_refptr<history::HistoryAddPageArgs> >
@@ -571,7 +572,8 @@ void
InstantLoader::TabContentsDelegateImpl::DidFailProvisionalLoad(
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) {
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) {
if (validated_url == loader_->url_) {
// This typically happens with downloads (which are disabled with
// instant active). To ensure the download happens when the user
presses
Index: chrome/browser/prerender/prerender_tab_helper.cc
diff --git a/chrome/browser/prerender/prerender_tab_helper.cc
b/chrome/browser/prerender/prerender_tab_helper.cc
index
8f01fefdeb4d851d97e1426af7e49098a89ebfbf..1b212e600a1fe80ced68438a07e62b423c72a656
100644
--- a/chrome/browser/prerender/prerender_tab_helper.cc
+++ b/chrome/browser/prerender/prerender_tab_helper.cc
@@ -43,7 +43,8 @@ PrerenderTabHelper::~PrerenderTabHelper() {

void PrerenderTabHelper::ProvisionalChangeToMainFrameUrl(
const GURL& url,
- const GURL& opener_url) {
+ const GURL& opener_url,
+ content::RenderViewHost* render_view_host) {
url_ = url;
RecordPageviewEvent(PAGEVIEW_EVENT_NEW_URL);
if (IsTopSite(url))
@@ -60,7 +61,8 @@ void PrerenderTabHelper::DidCommitProvisionalLoadForFrame(
int64 frame_id,
bool is_main_frame,
const GURL& validated_url,
- content::PageTransition transition_type) {
+ content::PageTransition transition_type,
+ content::RenderViewHost* render_view_host) {
if (!is_main_frame)
return;
PrerenderManager* prerender_manager = MaybeGetPrerenderManager();
Index: chrome/browser/prerender/prerender_tab_helper.h
diff --git a/chrome/browser/prerender/prerender_tab_helper.h
b/chrome/browser/prerender/prerender_tab_helper.h
index
808c1948808fab81cd5c6f47112504f73b0ce0ec..b364abd0ed6cfb689e3aba9f1378558c2d99292f
100644
--- a/chrome/browser/prerender/prerender_tab_helper.h
+++ b/chrome/browser/prerender/prerender_tab_helper.h
@@ -26,7 +26,8 @@ class PrerenderTabHelper : public
content::WebContentsObserver {
// content::WebContentsObserver implementation.
virtual void ProvisionalChangeToMainFrameUrl(
const GURL& url,
- const GURL& opener_url) OVERRIDE;
+ const GURL& opener_url,
+ content::RenderViewHost* render_view_host) OVERRIDE;
virtual void DidStopLoading() OVERRIDE;
virtual void DidStartProvisionalLoadForFrame(
int64 frame_id,
@@ -38,7 +39,8 @@ class PrerenderTabHelper : public
content::WebContentsObserver {
int64 frame_id,
bool is_main_frame,
const GURL& validated_url,
- content::PageTransition transition_type) OVERRIDE;
+ content::PageTransition transition_type,
+ content::RenderViewHost* render_view_host) OVERRIDE;
// Called when this prerendered TabContents has just been swapped in.
void PrerenderSwappedIn();

Index: chrome/browser/ssl/ssl_browser_tests.cc
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc
b/chrome/browser/ssl/ssl_browser_tests.cc
index
714a8297b76b23b8e2905ebdb91802e708e52fe2..d0c965324a234428a23116ddc6843273e894ada9
100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -60,7 +60,8 @@ class ProvisionalLoadWaiter : public
content::WebContentsObserver {
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) OVERRIDE {
+ const string16& error_description,
+ content::RenderViewHost* render_view_host) OVERRIDE {
seen_ = true;
if (waiting_)
MessageLoopForUI::current()->Quit();
Index:
content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
diff --git
a/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
b/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
index
a3565af138a728a47de4fcad1ca15d7fa206e03d..e99f0ca488108810f7e930b1366dc4b528ce2adc
100644
--- a/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
+++ b/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
@@ -175,7 +175,8 @@ void
BrowserPluginWebContentsObserver::DidCommitProvisionalLoadForFrame(
int64 frame_id,
bool is_main_frame,
const GURL& url,
- PageTransition transition_type) {
+ PageTransition transition_type,
+ content::RenderViewHost* render_view_host) {
typedef std::set<WebContentsImpl*> GuestSet;
GuestSet guests_to_delete;
for (GuestMap::const_iterator it = guests_.begin();
Index: content/browser/browser_plugin/browser_plugin_web_contents_observer.h
diff --git
a/content/browser/browser_plugin/browser_plugin_web_contents_observer.h
b/content/browser/browser_plugin/browser_plugin_web_contents_observer.h
index
99df72b88779c56e3508633ec60446a2258eafa3..2358083d0a941cc04e3fa280af77a69ed78cac26
100644
--- a/content/browser/browser_plugin/browser_plugin_web_contents_observer.h
+++ b/content/browser/browser_plugin/browser_plugin_web_contents_observer.h
@@ -47,7 +47,8 @@ class BrowserPluginWebContentsObserver: public
WebContentsObserver,
int64 frame_id,
bool is_main_frame,
const GURL& url,
- PageTransition transition_type) OVERRIDE;
+ PageTransition transition_type,
+ content::RenderViewHost* render_view_host) OVERRIDE;

virtual void RenderViewDeleted(
RenderViewHost* render_view_host) OVERRIDE;
Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc
b/content/browser/web_contents/web_contents_impl.cc
index
e8da542cbb626f156508f5b67334aa004b30d711..af72d311c4a98aa7104249c16200b5abf32a19b6
100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1524,7 +1524,8 @@ void WebContentsImpl::DidStartProvisionalLoadForFrame(
// Notify observers about the provisional change in the main frame URL.
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
ProvisionalChangeToMainFrameUrl(validated_url,
-
validated_opener_url));
+ validated_opener_url,
+ render_view_host));
}
}

@@ -1572,7 +1573,8 @@ void WebContentsImpl::DidRedirectProvisionalLoad(
// Notify observers about the provisional change in the main frame URL.
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
ProvisionalChangeToMainFrameUrl(validated_target_url,
- validated_opener_url));
+ validated_opener_url,
+ render_view_host));
}

void WebContentsImpl::DidFailProvisionalLoadWithError(
@@ -1640,7 +1642,8 @@ void WebContentsImpl::DidFailProvisionalLoadWithError(
params.is_main_frame,
validated_url,
params.error_code,
- params.error_description));
+ params.error_description,
+ render_view_host));
}

void WebContentsImpl::OnDidLoadResourceFromMemoryCache(
@@ -2177,7 +2180,7 @@ void WebContentsImpl::DidNavigate(
// Notify observers about the commit of the provisional load.
FOR_EACH_OBSERVER(WebContentsObserver, observers_,
DidCommitProvisionalLoadForFrame(params.frame_id,
- is_main_frame, params.url, transition_type));
+ is_main_frame, params.url, transition_type, rvh));
}

if (!did_navigate)
Index: content/public/browser/web_contents_observer.h
diff --git a/content/public/browser/web_contents_observer.h
b/content/public/browser/web_contents_observer.h
index
b0b77a8208da85face86592aa5f199321c926563..9791a86cbfa0ce7790b5731f1fe5018b8ce4722d
100644
--- a/content/public/browser/web_contents_observer.h
+++ b/content/public/browser/web_contents_observer.h
@@ -48,18 +48,22 @@ class CONTENT_EXPORT WebContentsObserver : public
IPC::Channel::Listener,
const GURL& validated_url,
bool is_error_page,
RenderViewHost* render_view_host) {}
- virtual void ProvisionalChangeToMainFrameUrl(const GURL& url,
- const GURL& opener_url) {}
+ virtual void ProvisionalChangeToMainFrameUrl(
+ const GURL& url,
+ const GURL& opener_url,
+ RenderViewHost* render_view_host) {}
virtual void DidCommitProvisionalLoadForFrame(
int64 frame_id,
bool is_main_frame,
const GURL& url,
- PageTransition transition_type) {}
+ PageTransition transition_type,
+ RenderViewHost* render_view_host) {}
virtual void DidFailProvisionalLoad(int64 frame_id,
bool is_main_frame,
const GURL& validated_url,
int error_code,
- const string16& error_description) {}
+ const string16& error_description,
+ RenderViewHost* render_view_host) {}
virtual void DocumentAvailableInMainFrame() {}
virtual void DocumentLoadedInFrame(int64 frame_id) {}
virtual void DidFinishLoad(int64 frame_id,


a...@chromium.org

unread,
May 13, 2012, 10:26:33 AM5/13/12
to joc...@chromium.org, cr...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
lgtm



https://chromiumcodereview.appspot.com/10392072/

cr...@chromium.org

unread,
May 14, 2012, 2:29:06 PM5/14/12
to joc...@chromium.org, a...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
LGTM. Do you have any plans yet for how to use that information?

Can you list 109792 as the bug number? I'd like to keep track of the cases
where we need to act on behalf of the correct renderer.

nit: now -> know in CL description.

https://chromiumcodereview.appspot.com/10392072/

commi...@chromium.org

unread,
May 16, 2012, 3:26:18 PM5/16/12
to joc...@chromium.org, a...@chromium.org, cr...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joc...@chromium.org/10392072/1


https://chromiumcodereview.appspot.com/10392072/

commi...@chromium.org

unread,
May 16, 2012, 3:26:25 PM5/16/12
to joc...@chromium.org, a...@chromium.org, cr...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
Can't apply patch for file
content/browser/browser_plugin/browser_plugin_web_contents_observer.cc.
While running patch -p1 --forward --force;
patching file
content/browser/browser_plugin/browser_plugin_web_contents_observer.cc
Hunk #1 FAILED at 175.
1 out of 1 hunk FAILED -- saving rejects to file
content/browser/browser_plugin/browser_plugin_web_contents_observer.cc.rej


https://chromiumcodereview.appspot.com/10392072/

commi...@chromium.org

unread,
May 16, 2012, 4:07:18 PM5/16/12
to joc...@chromium.org, a...@chromium.org, cr...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joc...@chromium.org/10392072/11001


https://chromiumcodereview.appspot.com/10392072/

commi...@chromium.org

unread,
May 16, 2012, 6:13:49 PM5/16/12
to joc...@chromium.org, a...@chromium.org, cr...@chromium.org, chromium...@chromium.org, cbentze...@chromium.org, mihaip-chro...@chromium.org, tburkar...@chromium.org, j...@chromium.org, joi+watc...@chromium.org, a...@chromium.org, dominic...@chromium.org, dari...@chromium.org, mme...@chromium.org
Change committed as 137534

https://chromiumcodereview.appspot.com/10392072/
Reply all
Reply to author
Forward
0 new messages