[DevTools] Fix crash in DevToolsWindow::Create

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 "chrome/browser/devtools/devtools_window.h"
 
 #include <algorithm>
 
 #include "base/json/json_reader.h"
 #include "base/metrics/histogram.h"
@@ skipping 376 matching lines @@
   return false;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::OpenDevToolsWindowForWorker(
     Profile* profile,
     const scoped_refptr<DevToolsAgentHost>& worker_agent) {
   DevToolsWindow* window = FindDevToolsWindow(worker_agent.get());
   if (!window) {
     window = DevToolsWindow::CreateDevToolsWindowForWorker(profile);
-    window->bindings_->AttachTo(worker_agent);
+    if (window)
+      window->bindings_->AttachTo(worker_agent);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());
   return window;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
     Profile* profile) {
   content::RecordAction(base::UserMetricsAction("DevTools_InspectWorker"));
   return Create(profile, GURL(), NULL, true, std::string(), false, "");
@@ skipping 30 matching lines @@
 }
 
 // static
 void DevToolsWindow::OpenExternalFrontend(
     Profile* profile,
     const std::string& frontend_url,
     const scoped_refptr<content::DevToolsAgentHost>& agent_host,
     bool isWorker) {
   DevToolsWindow* window = FindDevToolsWindow(agent_host.get());
   if (!window) {
-    window = Create(profile, GURL(), NULL, isWorker,
+    window = Create(profile, GURL(), nullptr, isWorker,
                     DevToolsUI::GetProxyURL(frontend_url).spec(), false, "");
-    window->bindings_->AttachTo(agent_host);
+    if (window)

[dgozman, 2015/01/29 15:50:35]

Let's only |if (window)| in this method, and |DCHECK(window)| in all other
places.

[vkuzkokov, 2015/01/30 12:44:15]

Done.

+      window->bindings_->AttachTo(agent_host);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());

[dgozman, 2015/01/29 15:50:35]

This |window| may be nullptr.

[vkuzkokov, 2015/01/30 12:44:15]

Done.

 }
 
 // static
 DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
     content::WebContents* inspected_web_contents,
     bool force_open,
     const DevToolsToggleAction& action,
     const std::string& settings) {
   scoped_refptr<DevToolsAgentHost> agent(
       DevToolsAgentHost::GetOrCreateFor(inspected_web_contents));
   DevToolsWindow* window = FindDevToolsWindow(agent.get());
   bool do_open = force_open;
   if (!window) {
     Profile* profile = Profile::FromBrowserContext(
         inspected_web_contents->GetBrowserContext());
     content::RecordAction(
         base::UserMetricsAction("DevTools_InspectRenderer"));
     window = Create(profile, GURL(), inspected_web_contents,
                     false, std::string(), true, settings);
+    if (!window)
+      return nullptr;
     window->bindings_->AttachTo(agent.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->is_docked_ || do_open)
@@ skipping 10 matching lines @@
     int x,
     int y) {
   scoped_refptr<DevToolsAgentHost> agent(
       DevToolsAgentHost::GetOrCreateFor(inspected_web_contents));
   agent->InspectElement(x, y);
   bool should_measure_time = FindDevToolsWindow(agent.get()) == NULL;
   base::TimeTicks start_time = base::TimeTicks::Now();
   // TODO(loislo): we should initiate DevTools window opening from within
   // renderer. Otherwise, we still can hit a race condition here.
   DevToolsWindow* window = OpenDevToolsWindow(inspected_web_contents);
-  if (should_measure_time)
+  if (window && should_measure_time)
     window->inspect_element_start_time_ = start_time;
 }
 
 void DevToolsWindow::ScheduleShow(const DevToolsToggleAction& action) {
   if (life_stage_ == kLoadCompleted) {
     Show(action);
     return;
   }
 
   // Action will be done only after load completed.
@@ skipping 152 matching lines @@
       intercepted_page_beforeunload_(false) {
   // Set up delegate, so we get fully-functional window immediately.
   // It will not appear in UI though until |life_stage_ == kLoadCompleted|.
   main_web_contents_->SetDelegate(this);
 
   main_web_contents_->GetController().LoadURL(
       DevToolsUIBindings::ApplyThemeToURL(profile, url), content::Referrer(),
       ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
 
   bindings_ = DevToolsUIBindings::ForWebContents(main_web_contents_);
-  DCHECK(bindings_);
+  if (!bindings_)
+    return;
 
   // Bindings take ownership over devtools as its delegate.
   bindings_->SetDelegate(this);
   // DevTools uses PageZoom::Zoom(), so main_web_contents_ requires a
   // ZoomController.
   ui_zoom::ZoomController::CreateForWebContents(main_web_contents_);
   ui_zoom::ZoomController::FromWebContents(main_web_contents_)
       ->SetShowsNotificationBubble(false);
 
   g_instances.Get().push_back(this);
@@ skipping 34 matching lines @@
         browser->is_type_popup()) {
       can_dock = false;
     }
   }
 
   // Create WebContents with devtools.
   GURL url(GetDevToolsURL(profile, frontend_url,
                           shared_worker_frontend,
                           remote_frontend,
                           can_dock, settings));
-  return new DevToolsWindow(profile, url, inspected_web_contents, can_dock);
+  DevToolsWindow* window =
+      new DevToolsWindow(profile, url, inspected_web_contents, can_dock);
+  if (!window->bindings_) {

[pfeldman, 2015/01/29 16:09:59]

Why is this happening? It makes code so much more complex.

[vkuzkokov, 2015/01/30 12:44:14]

DevToolsUI might be not created for some reason. Bad frontend_url from
remote device would do the trick.

+    delete window;
+    window = nullptr;
+  }
+  return window;
 }
 
 // static
 GURL DevToolsWindow::GetDevToolsURL(Profile* profile,
                                     const GURL& base_url,
                                     bool shared_worker_frontend,
                                     const std::string& remote_frontend,
                                     bool can_dock,
                                     const std::string& settings) {
   // Compatibility errors are encoded with data urls, pass them
@@ skipping 453 matching lines @@
       closure.Run();
     return;
   }
   load_completed_callback_ = closure;
 }
 
 bool DevToolsWindow::ForwardKeyboardEvent(
     const content::NativeWebKeyboardEvent& event) {
   return event_forwarder_->ForwardEvent(event);
 }