DevTools: Do not close devtools if there are dirty files in workspace

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/command_line.h"
 #include "base/json/json_writer.h"
@@ skipping 280 matching lines @@
   registry->RegisterBooleanPref(
       prefs::kDevToolsPortForwardingDefaultSet,
       false,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterDictionaryPref(
       prefs::kDevToolsPortForwardingConfig,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 // static
-DevToolsWindow* DevToolsWindow::GetDockedInstanceForInspectedTab(
+DevToolsWindow* DevToolsWindow::GetInstanceForInspectedTab(
     content::WebContents* inspected_web_contents) {
   if (!inspected_web_contents ||
       !DevToolsAgentHost::HasFor(inspected_web_contents->GetRenderViewHost()))
     return NULL;
   scoped_refptr<DevToolsAgentHost> agent(DevToolsAgentHost::GetOrCreateFor(
       inspected_web_contents->GetRenderViewHost()));
   DevToolsWindow* window = FindDevToolsWindow(agent.get());
+  return window ? window : NULL;

[vsevik, 2013/09/11 11:51:13]

return window

[lushnikov, 2013/09/11 14:03:09]

Done.

+}
+
+// static
+DevToolsWindow* DevToolsWindow::GetDockedInstanceForInspectedTab(
+    content::WebContents* inspected_web_contents) {
+  DevToolsWindow* window = GetInstanceForInspectedTab(inspected_web_contents);
   return (window && window->IsDocked()) ? window : NULL;
 }
 
 // static
 bool DevToolsWindow::IsDevToolsWindow(content::RenderViewHost* window_rvh) {
   return AsDevToolsWindow(window_rvh) != NULL;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::OpenDevToolsWindowForWorker(
@@ skipping 213 matching lines @@
     CreateDevToolsBrowser();
 
   if (should_show_window) {
     browser_->window()->Show();
     web_contents_->GetView()->SetInitialFocus();
   }
 
   ScheduleAction(action);
 }
 
+// static
+void DevToolsWindow::HandleBeforeUnload(content::WebContents* frontend_contents,
+    bool proceed, bool* proceed_to_fire_unload) {
+  DevToolsWindow* window = AsDevToolsWindow(
+      frontend_contents->GetRenderViewHost());
+  if (window)
+    window->BeforeUnloadFired(frontend_contents, proceed,
+        proceed_to_fire_unload);
+}
+
+bool DevToolsWindow::InspectedPageWillClose() {
+  if (inspected_page_is_closing_)
+    return false;
+  inspected_page_is_closing_ = true;
+  closing_target_ = DEVTOOLS_CLOSING_TARGET_INSPECTED_PAGE;
+  web_contents_->GetRenderViewHost()->FirePageBeforeUnload(false);
+  return true;
+}
+
+void DevToolsWindow::InspectedPageCancelClose() {
+  inspected_page_is_closing_ = false;
+}
+
 DevToolsWindow::DevToolsWindow(Profile* profile,
                                const GURL& url,
                                content::RenderViewHost* inspected_rvh,
                                DevToolsDockSide dock_side)
     : profile_(profile),
       browser_(NULL),
       dock_side_(dock_side),
       is_loaded_(false),
       action_on_load_(DEVTOOLS_TOGGLE_ACTION_SHOW),
       weak_factory_(this),
       width_(-1),
       height_(-1),
-      dock_side_before_minimized_(dock_side) {
+      dock_side_before_minimized_(dock_side),
+      closing_target_(DEVTOOLS_CLOSING_TARGET_NONE),
+      inspected_page_is_closing_(false) {
   web_contents_ =
       content::WebContents::Create(content::WebContents::CreateParams(profile));
   frontend_contents_observer_.reset(
       new FrontendWebContentsObserver(web_contents_));
 
   web_contents_->GetController().LoadURL(url, content::Referrer(),
       content::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
 
   frontend_host_.reset(content::DevToolsClientHost::CreateDevToolsFrontendHost(
       web_contents_, this));
@@ skipping 207 matching lines @@
                                     bool* was_blocked) {
   content::WebContents* inspected_web_contents = GetInspectedWebContents();
   if (inspected_web_contents) {
     inspected_web_contents->GetDelegate()->AddNewContents(
         source, new_contents, disposition, initial_pos, user_gesture,
         was_blocked);
   }
 }
 
 void DevToolsWindow::CloseContents(content::WebContents* source) {
+  // 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;
+}
+
+void DevToolsWindow::BeforeUnloadFired(content::WebContents* tab,
+                                       bool proceed,
+                                       bool* proceed_to_fire_unload) {
+  CHECK(closing_target_ != DEVTOOLS_CLOSING_TARGET_NONE);

[vsevik, 2013/09/11 11:51:13]

According to
http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-
you should not use CHECK here and below. Please use DCHECK.

[lushnikov, 2013/09/11 14:03:09]

Done.

+  if (closing_target_ == DEVTOOLS_CLOSING_TARGET_DEVTOOLS) {
+    if (proceed) {
+      content::DevToolsManager::GetInstance()->ClientHostClosing(
+          frontend_host_.get());
+    }
+    *proceed_to_fire_unload = proceed;
+  } else {
+    content::WebContents* inspected_web_contents = GetInspectedWebContents();
+    if (proceed) {
+      inspected_web_contents->GetRenderViewHost()->FirePageBeforeUnload(false);
+    } else {
+      bool tmp;
+      inspected_web_contents->GetDelegate()->BeforeUnloadFired(
+          inspected_web_contents, false, &tmp);
+      CHECK(!tmp);
+    }
+  }
 }
 
 bool DevToolsWindow::PreHandleKeyboardEvent(
     content::WebContents* source,
     const content::NativeWebKeyboardEvent& event,
     bool* is_keyboard_shortcut) {
   if (IsDocked()) {
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
     if (inspected_window) {
       return inspected_window->PreHandleKeyboardEvent(event,
@@ skipping 47 matching lines @@
   embedder_message_dispatcher_->Dispatch(message);
 }
 
 void DevToolsWindow::ActivateWindow() {
   if (IsDocked() && GetInspectedBrowserWindow())
     web_contents_->GetView()->Focus();
   else if (!IsDocked() && !browser_->window()->IsActive())
     browser_->window()->Activate();
 }
 
+void DevToolsWindow::ActivateContents(content::WebContents* contents) {

[vsevik, 2013/09/11 11:51:13]

I am not sure about this method, but why is it so different from the one above?

+  if (IsDocked()) {
+    content::WebContents* inspected_tab = this->GetInspectedWebContents();
+    inspected_tab->GetDelegate()->ActivateContents(inspected_tab);
+  } else {
+    browser_->window()->Activate();
+  }
+}
+
 void DevToolsWindow::CloseWindow() {
   DCHECK(IsDocked());
-  content::DevToolsManager::GetInstance()->ClientHostClosing(
-      frontend_host_.get());
-  Hide();
+  closing_target_ = DEVTOOLS_CLOSING_TARGET_DEVTOOLS;

[vsevik, 2013/09/11 11:51:13]

Looks like this field is redundant.

[lushnikov, 2013/09/11 14:03:09]

Done.

+  web_contents_->GetRenderViewHost()->FirePageBeforeUnload(false);
 }
 
 void DevToolsWindow::MoveWindow(int x, int y) {
   if (!IsDocked()) {
     gfx::Rect bounds = browser_->window()->GetBounds();
     bounds.Offset(x, y);
     browser_->window()->SetBounds(bounds);
   }
 }
 
@@ skipping 321 matching lines @@
 void DevToolsWindow::UpdateFrontendDockSide() {
   base::StringValue dock_side(SideToString(dock_side_));
   CallClientFunction("InspectorFrontendAPI.setDockSide", &dock_side, NULL,
                      NULL);
   base::FundamentalValue docked(IsDocked());
   CallClientFunction("InspectorFrontendAPI.setAttachedWindow", &docked, NULL,
                      NULL);
 }
 
 void DevToolsWindow::Hide() {
-  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;
-  } else {
-    // First, initiate self-destruct to free all the registrars.
-    // Then close all tabs. Browser will take care of deleting web_contents_
-    // for us.
-    Browser* browser = browser_;
-    delete this;
-    browser->tab_strip_model()->CloseAllTabs();
-  }
+  web_contents_->GetRenderViewHost()->ClosePage();

[vsevik, 2013/09/11 11:51:13]

IS this correct in case of undocked devtools?
why don't you delete this?
why don't we inline this method now that there is only one usage?
Please check beforeunload does not happen here.

[lushnikov, 2013/09/11 14:03:09]

The deletion of DevToolsWindow will happen in ::Observe method

 }
 
 void DevToolsWindow::ScheduleAction(DevToolsToggleAction action) {
   action_on_load_ = action;
   if (is_loaded_)
     DoAction();
 }
 
 void DevToolsWindow::DoAction() {
   UpdateFrontendDockSide();
@@ skipping 106 matching lines @@
 
 void DevToolsWindow::Restore() {
   if (dock_side_ == DEVTOOLS_DOCK_SIDE_MINIMIZED)
     SetDockSide(SideToString(dock_side_before_minimized_));
 }
 
 content::WebContents* DevToolsWindow::GetInspectedWebContents() {
   return inspected_contents_observer_ ?
       inspected_contents_observer_->web_contents() : NULL;
 }