Commit 7a0e557c authored by dgozman's avatar dgozman Committed by Commit bot

[DevTools] Calculate target type and title once in constructor

This is a logical thing to do by itself, improves performance and makes
it easier to reason about things.

It may also help to prevent crashes if we somehow get into inconsistent state
by not calling delegate methods at random times. Although not directly
fixing the linked crash, I expect it to go away.

BUG=704346
TBR=dtrainor@chromium.org

Review-Url: https://codereview.chromium.org/2799783006
Cr-Commit-Position: refs/heads/master@{#462959}
parent 36640ecf
......@@ -186,14 +186,6 @@ std::string DevToolsManagerDelegateAndroid::GetTargetType(
DevToolsAgentHost::kTypeOther;
}
std::string DevToolsManagerDelegateAndroid::GetTargetTitle(
content::RenderFrameHost* host) {
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(host);
TabAndroid* tab = TabAndroid::FromWebContents(web_contents);
return tab ? base::UTF16ToUTF8(tab->GetTitle()) : "";
}
bool DevToolsManagerDelegateAndroid::DiscoverTargets(
const DevToolsAgentHost::DiscoveryCallback& callback) {
#if defined(OS_ANDROID)
......
......@@ -27,7 +27,6 @@ class DevToolsManagerDelegateAndroid :
content::DevToolsAgentHost* agent_host,
base::DictionaryValue* command_dict) override;
std::string GetTargetType(content::RenderFrameHost* host) override;
std::string GetTargetTitle(content::RenderFrameHost* host) override;
bool DiscoverTargets(
const content::DevToolsAgentHost::DiscoveryCallback& callback) override;
scoped_refptr<content::DevToolsAgentHost> CreateNewTarget(
......
......@@ -34,10 +34,45 @@ char ChromeDevToolsManagerDelegate::kTypeApp[] = "app";
char ChromeDevToolsManagerDelegate::kTypeBackgroundPage[] = "background_page";
char ChromeDevToolsManagerDelegate::kTypeWebView[] = "webview";
namespace {
char kLocationsParam[] = "locations";
char kHostParam[] = "host";
char kPortParam[] = "port";
bool GetExtensionInfo(content::RenderFrameHost* host,
std::string* name,
std::string* type) {
content::WebContents* wc = content::WebContents::FromRenderFrameHost(host);
if (!wc)
return false;
Profile* profile = Profile::FromBrowserContext(wc->GetBrowserContext());
if (!profile)
return false;
const extensions::Extension* extension =
extensions::ProcessManager::Get(profile)->GetExtensionForRenderFrameHost(
host);
if (!extension)
return false;
extensions::ExtensionHost* extension_host =
extensions::ProcessManager::Get(profile)->GetBackgroundHostForExtension(
extension->id());
if (extension_host && extension_host->host_contents() == wc) {
*name = extension->name();
*type = ChromeDevToolsManagerDelegate::kTypeBackgroundPage;
return true;
} else if (extension->is_hosted_app() ||
extension->is_legacy_packaged_app() ||
extension->is_platform_app()) {
*name = extension->name();
*type = ChromeDevToolsManagerDelegate::kTypeApp;
return true;
}
return false;
}
} // namespace
class ChromeDevToolsManagerDelegate::HostData {
public:
HostData() {}
......@@ -103,47 +138,20 @@ std::string ChromeDevToolsManagerDelegate::GetTargetType(
return DevToolsAgentHost::kTypePage;
}
const extensions::Extension* extension = extensions::ExtensionRegistry::Get(
web_contents->GetBrowserContext())->enabled_extensions().GetByID(
host->GetLastCommittedURL().host());
if (!extension)
std::string extension_name;
std::string extension_type;
if (!GetExtensionInfo(host, &extension_name, &extension_type))
return DevToolsAgentHost::kTypeOther;
Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
if (!profile)
return DevToolsAgentHost::kTypeOther;
extensions::ExtensionHost* extension_host =
extensions::ProcessManager::Get(profile)
->GetBackgroundHostForExtension(extension->id());
if (extension_host &&
extension_host->host_contents() == web_contents) {
return kTypeBackgroundPage;
} else if (extension->is_hosted_app()
|| extension->is_legacy_packaged_app()
|| extension->is_platform_app()) {
return kTypeApp;
}
return DevToolsAgentHost::kTypeOther;
return extension_type;
}
std::string ChromeDevToolsManagerDelegate::GetTargetTitle(
content::RenderFrameHost* host) {
content::WebContents* web_contents =
content::WebContents::FromRenderFrameHost(host);
if (host->GetParent())
return host->GetLastCommittedURL().spec();
for (TabContentsIterator it; !it.done(); it.Next()) {
if (*it == web_contents)
return base::UTF16ToUTF8(web_contents->GetTitle());
}
const extensions::Extension* extension = extensions::ExtensionRegistry::Get(
web_contents->GetBrowserContext())->enabled_extensions().GetByID(
host->GetLastCommittedURL().host());
if (extension)
return extension->name();
return "";
std::string extension_name;
std::string extension_type;
if (!GetExtensionInfo(host, &extension_name, &extension_type))
return std::string();
return extension_name;
}
scoped_refptr<DevToolsAgentHost>
......
......@@ -462,6 +462,13 @@ RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
g_instances.Get().push_back(this);
AddRef(); // Balanced in RenderFrameHostDestroyed.
DevToolsManager* manager = DevToolsManager::GetInstance();
if (manager->delegate()) {
type_ = manager->delegate()->GetTargetType(host);
title_ = manager->delegate()->GetTargetTitle(host);
}
NotifyCreated();
}
......@@ -1015,24 +1022,18 @@ std::string RenderFrameDevToolsAgentHost::GetParentId() {
}
std::string RenderFrameDevToolsAgentHost::GetType() {
DevToolsManager* manager = DevToolsManager::GetInstance();
if (manager->delegate() && current_) {
std::string result = manager->delegate()->GetTargetType(current_->host());
if (!result.empty())
return result;
}
if (!type_.empty())
return type_;
if (IsChildFrame())
return kTypeFrame;
return kTypePage;
}
std::string RenderFrameDevToolsAgentHost::GetTitle() {
DevToolsManager* manager = DevToolsManager::GetInstance();
if (manager->delegate() && current_) {
std::string result = manager->delegate()->GetTargetTitle(current_->host());
if (!result.empty())
return result;
}
if (!title_.empty())
return title_;
if (current_ && current_->host()->GetParent())
return current_->host()->GetLastCommittedURL().spec();
content::WebContents* web_contents = GetWebContents();
if (web_contents)
return base::UTF16ToUTF8(web_contents->GetTitle());
......
......@@ -171,6 +171,8 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
#endif
RenderFrameHostImpl* handlers_frame_host_;
bool current_frame_crashed_;
std::string title_;
std::string type_;
// PlzNavigate
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment