Avoid retaining the pointer to inspected WebContents in DevToolsWindow

chrome/browser/devtools/devtools_window.cc

Index: chrome/browser/devtools/devtools_window.cc
diff --git a/chrome/browser/devtools/devtools_window.cc b/chrome/browser/devtools/devtools_window.cc
index 0123c053cf61eac1699904e6c08a09a59847df87..2d1dc34010df49d1191ccb7995716228daa18548 100644
--- a/chrome/browser/devtools/devtools_window.cc
+++ b/chrome/browser/devtools/devtools_window.cc
@@ -164,7 +164,7 @@ DevToolsWindow* DevToolsWindow::OpenDevToolsWindowForWorker(
 // static
 DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
     Profile* profile) {
-  return Create(profile, NULL, DEVTOOLS_DOCK_SIDE_UNDOCKED, true);
+  return Create(profile, DEVTOOLS_DOCK_SIDE_UNDOCKED, true);
 }
 
 // static
@@ -203,7 +203,6 @@ void DevToolsWindow::InspectElement(RenderViewHost* inspected_rvh,
 
 DevToolsWindow* DevToolsWindow::Create(
     Profile* profile,
-    RenderViewHost* inspected_rvh,
     DevToolsDockSide dock_side,
     bool shared_worker_frontend) {
   // Create WebContents with devtools.
@@ -220,15 +219,13 @@ DevToolsWindow* DevToolsWindow::Create(
   content::ChildProcessSecurityPolicy::GetInstance()->GrantScheme(
       process_id, chrome::kFileScheme);
   content::DevToolsClientHost::SetupDevToolsFrontendClient(render_view_host);
-  return new DevToolsWindow(web_contents, profile, inspected_rvh, dock_side);
+  return new DevToolsWindow(web_contents, profile, dock_side);
 }
 
 DevToolsWindow::DevToolsWindow(WebContents* web_contents,
                                Profile* profile,
-                               RenderViewHost* inspected_rvh,
                                DevToolsDockSide dock_side)
     : profile_(profile),
-      inspected_web_contents_(NULL),
       web_contents_(web_contents),
       browser_(NULL),
       dock_side_(dock_side),
@@ -261,9 +258,6 @@ DevToolsWindow::DevToolsWindow(WebContents* web_contents,
       chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
       content::Source<ThemeService>(
           ThemeServiceFactory::GetForProfile(profile_)));
-  // There is no inspected_rvh in case of shared workers.
-  if (inspected_rvh)
-    inspected_web_contents_ = WebContents::FromRenderViewHost(inspected_rvh);
 }
 
 DevToolsWindow::~DevToolsWindow() {
@@ -275,6 +269,19 @@ DevToolsWindow::~DevToolsWindow() {
   instances.erase(it);
 }
 
+content::WebContents* DevToolsWindow::GetInspectedWebContents() {
+  DevToolsAgentHost* agent_host = DevToolsManager::GetInstance()->
+      GetDevToolsAgentHostFor(frontend_host_.get());
+  if (!agent_host)
+    return NULL;
+
+  RenderViewHost* rvh = agent_host->GetRenderViewHost();
+  if (!rvh)
+    return NULL;
+
+  return WebContents::FromRenderViewHost(rvh);
+}
+
 void DevToolsWindow::InspectedContentsClosing() {
   if (IsDocked()) {
     // Update dev tools to reflect removed dev tools window.
@@ -421,12 +428,12 @@ void DevToolsWindow::CreateDevToolsBrowser() {
 
 bool DevToolsWindow::FindInspectedBrowserAndTabIndex(Browser** browser,
                                                      int* tab) {
-  if (!inspected_web_contents_)
+  if (!GetInspectedWebContents())

[yurys, 2013/03/07 13:24:49]

Consider using local variable for the result here and in other places below.

     return false;
 
   for (chrome::BrowserIterator it; !it.done(); it.Next()) {
     int tab_index = it->tab_strip_model()->GetIndexOfWebContents(
-        inspected_web_contents_);
+        GetInspectedWebContents());
     if (tab_index != TabStripModel::kNoTab) {
       *browser = *it;
       *tab = tab_index;
@@ -461,9 +468,9 @@ void DevToolsWindow::UpdateFrontendDockSide() {
 
 
 void DevToolsWindow::AddDevToolsExtensionsToClient() {
-  if (inspected_web_contents_) {
+  if (GetInspectedWebContents()) {
     SessionTabHelper* session_tab_helper =
-        SessionTabHelper::FromWebContents(inspected_web_contents_);
+        SessionTabHelper::FromWebContents(GetInspectedWebContents());
     if (session_tab_helper) {
       base::FundamentalValue tabId(session_tab_helper->session_id().id());
       CallClientFunction("WebInspector.setInspectedTabId", &tabId);
@@ -509,8 +516,8 @@ WebContents* DevToolsWindow::OpenURLFromTab(WebContents* source,
     return nav_params.target_contents;
   }
 
-  if (inspected_web_contents_)
-    return inspected_web_contents_->OpenURL(params);
+  if (GetInspectedWebContents())
+    return GetInspectedWebContents()->OpenURL(params);
   return NULL;
 }
 
@@ -637,8 +644,8 @@ void DevToolsWindow::AddNewContents(WebContents* source,
                                     const gfx::Rect& initial_pos,
                                     bool user_gesture,
                                     bool* was_blocked) {
-  if (inspected_web_contents_) {
-    inspected_web_contents_->GetDelegate()->AddNewContents(
+  if (GetInspectedWebContents()) {
+    GetInspectedWebContents()->GetDelegate()->AddNewContents(
         source, new_contents, disposition, initial_pos, user_gesture,
         was_blocked);
   }
@@ -690,7 +697,7 @@ DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
     Profile* profile = Profile::FromBrowserContext(
         inspected_rvh->GetProcess()->GetBrowserContext());
     DevToolsDockSide dock_side = GetDockSideFromPrefs(profile);
-    window = Create(profile, inspected_rvh, dock_side, false);
+    window = Create(profile, dock_side, false);
     manager->RegisterDevToolsClientHostFor(agent, window->frontend_host_.get());
     do_open = true;
   }
@@ -777,7 +784,7 @@ void DevToolsWindow::SetDockSide(const std::string& side) {
   bool dock_requested = requested_side != DEVTOOLS_DOCK_SIDE_UNDOCKED;
   bool is_docked = IsDocked();
 
-  if (dock_requested && (!inspected_web_contents_ ||
+  if (dock_requested && (!GetInspectedWebContents() ||
       !GetInspectedBrowserWindow() || IsInspectedBrowserPopupOrPanel())) {
       // Cannot dock, avoid window flashing due to close-reopen cycle.
     return;
@@ -823,8 +830,8 @@ void DevToolsWindow::OpenInNewTab(const std::string& url) {
                        NEW_FOREGROUND_TAB,
                        content::PAGE_TRANSITION_LINK,
                        false /* is_renderer_initiated */);
-  if (inspected_web_contents_) {
-    inspected_web_contents_->OpenURL(params);
+  if (GetInspectedWebContents()) {
+    GetInspectedWebContents()->OpenURL(params);
   } else {
     chrome::HostDesktopType host_desktop_type;
     if (browser_) {
@@ -931,8 +938,8 @@ void DevToolsWindow::FileSystemAdded(
 }
 
 content::JavaScriptDialogManager* DevToolsWindow::GetJavaScriptDialogManager() {
-  if (inspected_web_contents_ && inspected_web_contents_->GetDelegate()) {
-    return inspected_web_contents_->GetDelegate()->
+  if (GetInspectedWebContents() && GetInspectedWebContents()->GetDelegate()) {
+    return GetInspectedWebContents()->GetDelegate()->
         GetJavaScriptDialogManager();
   }
   return content::WebContentsDelegate::GetJavaScriptDialogManager();
@@ -954,11 +961,11 @@ void DevToolsWindow::WebContentsFocused(WebContents* contents) {
 }
 
 void DevToolsWindow::UpdateBrowserToolbar() {
-  if (!inspected_web_contents_)
+  if (!GetInspectedWebContents())
     return;
   BrowserWindow* inspected_window = GetInspectedBrowserWindow();
   if (inspected_window)
-    inspected_window->UpdateToolbar(inspected_web_contents_, false);
+    inspected_window->UpdateToolbar(GetInspectedWebContents(), false);
 }
 
 bool DevToolsWindow::IsDocked() {