Avoid retaining the pointer to inspected WebContents in DevToolsWindow

chrome/browser/devtools/devtools_window.cc

 // Copyright (c) 2012 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 <algorithm>
 
 #include "base/command_line.h"
 #include "base/json/json_writer.h"
 #include "base/lazy_instance.h"
 #include "base/stringprintf.h"
@@ skipping 146 matching lines @@
         worker_agent,
         window->frontend_host_.get());
   }
   window->Show(DEVTOOLS_TOGGLE_ACTION_SHOW);
   return window;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
     Profile* profile) {
-  return Create(profile, NULL, DEVTOOLS_DOCK_SIDE_UNDOCKED, true);
+  return Create(profile, DEVTOOLS_DOCK_SIDE_UNDOCKED, true);
 }
 
 // static
 DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
     RenderViewHost* inspected_rvh) {
   return ToggleDevToolsWindow(inspected_rvh, true,
                               DEVTOOLS_TOGGLE_ACTION_SHOW);
 }
 
 // static
@@ skipping 18 matching lines @@
   scoped_refptr<DevToolsAgentHost> agent(
       DevToolsAgentHost::GetFor(inspected_rvh));
   DevToolsManager::GetInstance()->InspectElement(agent, x, y);
   // TODO(loislo): we should initiate DevTools window opening from within
   // renderer. Otherwise, we still can hit a race condition here.
   OpenDevToolsWindow(inspected_rvh);
 }
 
 DevToolsWindow* DevToolsWindow::Create(
     Profile* profile,
-    RenderViewHost* inspected_rvh,
     DevToolsDockSide dock_side,
     bool shared_worker_frontend) {
   // Create WebContents with devtools.
   WebContents* web_contents =
       WebContents::Create(WebContents::CreateParams(profile));
   web_contents->GetController().LoadURL(
       GetDevToolsUrl(profile, dock_side, shared_worker_frontend),
       content::Referrer(),
       content::PAGE_TRANSITION_AUTO_TOPLEVEL,
       std::string());
 
   RenderViewHost* render_view_host = web_contents->GetRenderViewHost();
   int process_id = render_view_host->GetProcess()->GetID();
   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),
       is_loaded_(false),
       action_on_load_(DEVTOOLS_TOGGLE_ACTION_SHOW),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       width_(-1),
       height_(-1) {
   frontend_host_.reset(
       DevToolsClientHost::CreateDevToolsFrontendHost(web_contents, this));
@@ skipping 12 matching lines @@
       content::Source<NavigationController>(&web_contents->GetController()));
   registrar_.Add(
       this,
       chrome::NOTIFICATION_TAB_CLOSING,
       content::Source<NavigationController>(&web_contents->GetController()));
   registrar_.Add(
       this,
       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() {
   DevToolsWindowList& instances = g_instances.Get();
   DevToolsWindowList::iterator it = std::find(instances.begin(),
                                               instances.end(),
                                               this);
   DCHECK(it != instances.end());
   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.
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
     if (inspected_window)
       inspected_window->UpdateDevTools();
     // In case of docked web_contents_, we own it so delete here.
     delete web_contents_;
 
     delete this;
@@ skipping 126 matching lines @@
 
   browser_ = new Browser(Browser::CreateParams::CreateForDevTools(
                              profile_, host_desktop_type));
   browser_->tab_strip_model()->AddWebContents(
       web_contents_, -1, content::PAGE_TRANSITION_AUTO_TOPLEVEL,
       TabStripModel::ADD_ACTIVE);
 }
 
 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;
       return true;
     }
   }
   return false;
 }
 
 BrowserWindow* DevToolsWindow::GetInspectedBrowserWindow() {
@@ skipping 14 matching lines @@
 
 void DevToolsWindow::UpdateFrontendDockSide() {
   base::StringValue dock_side(SideToString(dock_side_));
   CallClientFunction("InspectorFrontendAPI.setDockSide", &dock_side);
   base::FundamentalValue docked(IsDocked());
   CallClientFunction("InspectorFrontendAPI.setAttachedWindow", &docked);
 }
 
 
 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);
     }
   }
   ListValue results;
   Profile* profile =
       Profile::FromBrowserContext(web_contents_->GetBrowserContext());
   const ExtensionService* extension_service = extensions::ExtensionSystem::Get(
       profile->GetOriginalProfile())->extension_service();
@@ skipping 25 matching lines @@
     chrome::NavigateParams nav_params(profile_, params.url, params.transition);
     FillNavigateParamsFromOpenURLParams(&nav_params, params);
     nav_params.source_contents = source;
     nav_params.tabstrip_add_types = TabStripModel::ADD_NONE;
     nav_params.window_action = chrome::NavigateParams::SHOW_WINDOW;
     nav_params.user_gesture = true;
     chrome::Navigate(&nav_params);
     return nav_params.target_contents;
   }
 
-  if (inspected_web_contents_)
-    return inspected_web_contents_->OpenURL(params);
+  if (GetInspectedWebContents())
+    return GetInspectedWebContents()->OpenURL(params);
   return NULL;
 }
 
 void DevToolsWindow::CallClientFunction(const std::string& function_name,
                                         const Value* arg1,
                                         const Value* arg2) {
   std::string params;
   if (arg1) {
     std::string json;
     base::JSONWriter::Write(arg1, &json);
@@ skipping 106 matching lines @@
   web_contents_->GetRenderViewHost()->
       ExecuteJavascriptInWebFrame(string16(), UTF8ToUTF16(command));
 }
 
 void DevToolsWindow::AddNewContents(WebContents* source,
                                     WebContents* new_contents,
                                     WindowOpenDisposition disposition,
                                     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);
   }
 }
 
 bool DevToolsWindow::PreHandleKeyboardEvent(
     WebContents* source,
     const NativeWebKeyboardEvent& event, bool* is_keyboard_shortcut) {
   if (IsDocked()) {
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
@@ skipping 31 matching lines @@
     // Break remote debugging / extension debugging session.
     host->ReplacedWithAnotherClient();
     manager->UnregisterDevToolsClientHostFor(agent);
   }
 
   bool do_open = force_open;
   if (!window) {
     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;
   }
 
   // Update toolbar to reflect DevTools changes.
   window->UpdateBrowserToolbar();
 
   // If window is docked and visible, we hide it on toggle. If window is
   // undocked, we show (activate) it.
   if (!window->IsDocked() || do_open)
@@ skipping 66 matching lines @@
     bounds.Offset(x, y);
     browser_->window()->SetBounds(bounds);
   }
 }
 
 void DevToolsWindow::SetDockSide(const std::string& side) {
   DevToolsDockSide requested_side = SideFromString(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;
   }
 
   dock_side_ = requested_side;
   if (dock_requested) {
     if (!is_docked) {
       // Detach window from the external devtools browser. It will lead to
       // the browser object's close and delete. Remove observer first.
@@ skipping 25 matching lines @@
 
   Show(DEVTOOLS_TOGGLE_ACTION_SHOW);
 }
 
 void DevToolsWindow::OpenInNewTab(const std::string& url) {
   OpenURLParams params(GURL(url),
                        content::Referrer(),
                        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_) {
       host_desktop_type = browser_->host_desktop_type();
     } else {
       // There should always be a browser when there are no inspected web
       // contents.
       NOTREACHED();
       host_desktop_type = chrome::GetActiveDesktop();
     }
@@ skipping 86 matching lines @@
   if (!file_system.file_system_path.empty())
     file_system_value = CreateFileSystemValue(file_system);
   CallClientFunction("InspectorFrontendAPI.fileSystemAdded",
                      &error_string_value,
                      file_system_value);
   if (file_system_value)
     delete file_system_value;
 }
 
 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();
 }
 
 void DevToolsWindow::RunFileChooser(WebContents* web_contents,
                                     const FileChooserParams& params) {
   FileSelectHelper::RunFileChooser(web_contents, params);
 }
 
 void DevToolsWindow::WebContentsFocused(WebContents* contents) {
   Browser* inspected_browser = NULL;
   int inspected_tab_index = -1;
 
   if (IsDocked() && FindInspectedBrowserAndTabIndex(&inspected_browser,
                                                     &inspected_tab_index)) {
     inspected_browser->window()->WebContentsFocused(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() {
   return dock_side_ != DEVTOOLS_DOCK_SIDE_UNDOCKED;
 }
 
 // static
 DevToolsDockSide DevToolsWindow::GetDockSideFromPrefs(Profile* profile) {
   std::string dock_side =
       profile->GetPrefs()->GetString(prefs::kDevToolsDockSide);
@@ skipping 28 matching lines @@
 
 // static
 DevToolsDockSide DevToolsWindow::SideFromString(
     const std::string& dock_side) {
   if (dock_side == kDockSideRight)
     return DEVTOOLS_DOCK_SIDE_RIGHT;
   if (dock_side == kDockSideBottom)
     return DEVTOOLS_DOCK_SIDE_BOTTOM;
   return DEVTOOLS_DOCK_SIDE_UNDOCKED;
 }