Removing callers of the WebContents::GetRenderProcessHost() method.
WebContents::GetRenderProcessHost is an API that doesn't behave as
expected with out-of-process iframes, as multiple processes can be
associated with a single WebContents. This CL replaces callers of this
API, so that they go through an equivalent API that explicitly selects
which frame's process is needed.
This CL was started by using an ad-hoc clang-plugin that replaced
existing callsites of WebContents::GetRenderProcessHost() with a call to
something like wc->GetMainFrame()->GetProcess(). This was followed-up
by manually adding an include of render_frame_host.h and git cl format
and git cl lint and then a self-review with small tweaks (e.g. sometimes
using a wc->GetRenderViewHost()->GetProcess() is more appropriate).
This CL was uploaded by git cl split.
[email protected]
Bug: 666525
Change-Id: Iff2b7f18d4c593b10846eac3c8ec5a03a0b0ef6d
Reviewed-on: https://chromium-review.googlesource.com/689114
Reviewed-by: Avi Drissman <[email protected]>
Reviewed-by: Marc Treib <[email protected]>
Commit-Queue: Łukasz Anforowicz <[email protected]>
Cr-Commit-Position: refs/heads/master@{#505408}diff --git a/chrome/browser/search/search.cc b/chrome/browser/search/search.cc
index c8a1229..9bfab12 100644
--- a/chrome/browser/search/search.cc
+++ b/chrome/browser/search/search.cc
@@ -25,6 +25,7 @@
#include "components/search_engines/search_engine_type.h"
#include "components/search_engines/template_url_service.h"
#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
@@ -203,7 +204,7 @@
bool IsRenderedInInstantProcess(const content::WebContents* contents,
Profile* profile) {
const content::RenderProcessHost* process_host =
- contents->GetRenderProcessHost();
+ contents->GetMainFrame()->GetProcess();
if (!process_host)
return false;
diff --git a/chrome/browser/search/search_unittest.cc b/chrome/browser/search/search_unittest.cc
index b6b366c4..74fa2a9 100644
--- a/chrome/browser/search/search_unittest.cc
+++ b/chrome/browser/search/search_unittest.cc
@@ -4,6 +4,9 @@
#include <stddef.h>
+#include <map>
+#include <memory>
+
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/strings/utf_string_conversions.h"
@@ -22,6 +25,7 @@
#include "components/search/search.h"
#include "components/search_engines/template_url_service.h"
#include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/site_instance.h"
@@ -69,7 +73,7 @@
InstantService* instant_service =
InstantServiceFactory::GetForProfile(profile());
return instant_service->IsInstantProcess(
- contents->GetRenderProcessHost()->GetID());
+ contents->GetMainFrame()->GetProcess()->GetID());
}
};
@@ -184,7 +188,7 @@
const scoped_refptr<content::SiteInstance> start_site_instance =
contents->GetSiteInstance();
const content::RenderProcessHost* start_rph =
- contents->GetRenderProcessHost();
+ contents->GetMainFrame()->GetProcess();
const content::RenderViewHost* start_rvh =
contents->GetRenderViewHost();
@@ -200,7 +204,7 @@
start_rvh == contents->GetRenderViewHost())
<< test.description;
EXPECT_EQ(test.same_site_instance,
- start_rph == contents->GetRenderProcessHost())
+ start_rph == contents->GetMainFrame()->GetProcess())
<< test.description;
}
}
@@ -221,7 +225,7 @@
const scoped_refptr<content::SiteInstance> start_site_instance =
contents->GetSiteInstance();
const content::RenderProcessHost* start_rph =
- contents->GetRenderProcessHost();
+ contents->GetMainFrame()->GetProcess();
const content::RenderViewHost* start_rvh =
contents->GetRenderViewHost();
@@ -244,7 +248,7 @@
start_rvh == contents->GetRenderViewHost())
<< test.description;
EXPECT_EQ(test.same_site_instance,
- start_rph == contents->GetRenderProcessHost())
+ start_rph == contents->GetMainFrame()->GetProcess())
<< test.description;
}
}
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 305bfdf..894f77e 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -937,7 +937,7 @@
return host ? host->GetProcess() : NULL;
}
-RenderFrameHostImpl* WebContentsImpl::GetMainFrame() {
+RenderFrameHostImpl* WebContentsImpl::GetMainFrame() const {
return frame_tree_.root()->current_frame_host();
}
@@ -5821,7 +5821,7 @@
int active_match_ordinal,
bool final_update) {
if (delegate_ && !is_being_destroyed_ &&
- !GetRenderProcessHost()->FastShutdownStarted()) {
+ !GetMainFrame()->GetProcess()->FastShutdownStarted()) {
delegate_->FindReply(this, request_id, number_of_matches, selection_rect,
active_match_ordinal, final_update);
}
@@ -5847,7 +5847,7 @@
return;
}
// Notify for UI updates if the state changes.
- DCHECK(bluetooth_connected_device_count_ != 0);
+ DCHECK_NE(bluetooth_connected_device_count_, 0u);
bluetooth_connected_device_count_--;
if (bluetooth_connected_device_count_ == 0) {
NotifyNavigationStateChanged(INVALIDATE_TYPE_TAB);
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 27058285..7479d51 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -12,6 +12,8 @@
#include <memory>
#include <set>
#include <string>
+#include <utility>
+#include <vector>
#include "base/compiler_specific.h"
#include "base/gtest_prod_util.h"
@@ -305,7 +307,7 @@
const GURL& GetVisibleURL() const override;
const GURL& GetLastCommittedURL() const override;
RenderProcessHost* GetRenderProcessHost() const override;
- RenderFrameHostImpl* GetMainFrame() override;
+ RenderFrameHostImpl* GetMainFrame() const override;
RenderFrameHostImpl* GetFocusedFrame() override;
RenderFrameHostImpl* FindFrameByFrameTreeNodeId(int frame_tree_node_id,
int process_id) override;
@@ -1016,7 +1018,7 @@
};
// See WebContents::Create for a description of these parameters.
- WebContentsImpl(BrowserContext* browser_context);
+ explicit WebContentsImpl(BrowserContext* browser_context);
// Add and remove observers for page navigation notifications. The order in
// which notifications are sent to observers is undefined. Clients must be
diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
index 61e22be..74b7cb1 100644
--- a/content/public/browser/web_contents.h
+++ b/content/public/browser/web_contents.h
@@ -9,6 +9,8 @@
#include <memory>
#include <set>
+#include <string>
+#include <vector>
#include "base/callback_forward.h"
#include "base/files/file_path.h"
@@ -248,7 +250,7 @@
virtual RenderProcessHost* GetRenderProcessHost() const = 0;
// Returns the main frame for the currently active view.
- virtual RenderFrameHost* GetMainFrame() = 0;
+ virtual RenderFrameHost* GetMainFrame() const = 0;
// Returns the focused frame for the currently active view.
virtual RenderFrameHost* GetFocusedFrame() = 0;
@@ -740,8 +742,9 @@
// Returns true if the WebContents is responsible for displaying a subframe
// in a different process from its parent page.
- // TODO: this doesn't really belong here. With site isolation, this should be
- // removed since we can then embed iframes in different processes.
+ // TODO(lazyboy): https://crbug.com/542893: this doesn't really belong here.
+ // With site isolation, this should be removed since we can then embed iframes
+ // in different processes.
virtual bool IsSubframe() const = 0;
// Finds text on a page. |search_text| should not be empty.
diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc
index a4c79dba..1cafe12 100644
--- a/content/test/test_web_contents.cc
+++ b/content/test/test_web_contents.cc
@@ -4,7 +4,9 @@
#include "content/test/test_web_contents.h"
+#include <memory>
#include <utility>
+#include <vector>
#include "content/browser/browser_url_handler_impl.h"
#include "content/browser/frame_host/cross_process_frame_connector.h"
@@ -51,7 +53,7 @@
EXPECT_FALSE(expect_set_history_offset_and_length_);
}
-TestRenderFrameHost* TestWebContents::GetMainFrame() {
+TestRenderFrameHost* TestWebContents::GetMainFrame() const {
return static_cast<TestRenderFrameHost*>(WebContentsImpl::GetMainFrame());
}
diff --git a/content/test/test_web_contents.h b/content/test/test_web_contents.h
index 899e5e3..0fc604e 100644
--- a/content/test/test_web_contents.h
+++ b/content/test/test_web_contents.h
@@ -10,6 +10,7 @@
#include <list>
#include <map>
#include <string>
+#include <utility>
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/test/web_contents_tester.h"
@@ -47,7 +48,7 @@
scoped_refptr<SiteInstance> instance);
// WebContentsImpl overrides (returning the same values, but in Test* types)
- TestRenderFrameHost* GetMainFrame() override;
+ TestRenderFrameHost* GetMainFrame() const override;
TestRenderViewHost* GetRenderViewHost() const override;
// Overrides to avoid establishing Mojo connection with renderer process.
int DownloadImage(const GURL& url,
