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

chrome/browser/ui/fast_unload_controller.cc

 // Copyright 2013 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/ui/fast_unload_controller.h"
 
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/devtools_window.h"
@@ skipping 53 matching lines @@
 }
 
 bool FastUnloadController::CanCloseContents(content::WebContents* contents) {
   // Don't try to close the tab when the whole browser is being closed, since
   // that avoids the fast shutdown path where we just kill all the renderers.
   return !is_attempting_to_close_browser_ ||
       is_calling_before_unload_handlers();
 }
 
 // static
+bool FastUnloadController::CanFastShutdownWebContents(
+    content::WebContents* contents) {

[jeremy, 2013/11/07 14:18:34]

Can you please provide a comment on what fastshutdown means exactly and point to
it from the various delegates.

[lushnikov, 2013/11/07 17:18:10]

Do you think this one is needed? Looks like it could be easily figured out from
the code - there's only a single call of this method, which brings us right to
the fast shutdown code in content layer.

[jeremy, 2013/11/10 12:50:29]

There are way too many moving parts here and for someone unfamiliar with the
code this is very hard to follow.   Having a descriptive method name helps
reduce the cognitive load when reading through the code.

I'm sure we can find a generic term which has nothing to do with DevTools but
conveys what's going on, how about something like BeforeUnloadInterceptNeeded()
or somesunch?

+  return DevToolsWindow::GetInstanceForInspectedRenderViewHost(
+      contents->GetRenderViewHost()) == NULL;
+}
+
+// static
 bool FastUnloadController::RunUnloadEventsHelper(
     content::WebContents* contents) {
+  // If there's a devtools window attached to the web contents,
+  // then we would like to run its beforeunload handlers first.

[jeremy, 2013/11/07 14:18:34]

Please point to the comment that explains why.

[lushnikov, 2013/11/07 17:18:10]

Done.

+  if (DevToolsWindow::InterceptPageBeforeUnload(contents)) {

[jeremy, 2013/11/10 12:50:29]

2 things are unclear from this callsite:
* Does InterceptPageBeforeUnload() actually display the dialog?
* Is this a sync or an async operation?

Suggest changing the function name and adding a comment to make this clear.

+    return true;
+  }
   // If the WebContents is not connected yet, then there's no unload
   // handler we can fire even if the WebContents has an unload listener.
   // One case where we hit this is in a tab that has an infinite loop
   // before load.
   if (contents->NeedToFireBeforeUnload()) {
     // If the page has unload listeners, then we tell the renderer to fire
     // them. Once they have fired, we'll get a message back saying whether
     // to proceed closing the page or not, which sends us back to this method
     // with the NeedToFireBeforeUnload bit cleared.
     contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     return true;
   }
   return false;
 }
 
 bool FastUnloadController::BeforeUnloadFired(content::WebContents* contents,
                                              bool proceed) {
+  if (!proceed)
+    DevToolsWindow::PageClosingCanceled(contents);
+
   if (!is_attempting_to_close_browser_) {
     if (!proceed) {
       contents->SetClosedByUserGesture(false);
     } else {
       // No more dialogs are possible, so remove the tab and finish
       // running unload listeners asynchrounously.
       browser_->tab_strip_model()->delegate()->CreateHistoricalTab(contents);
       DetachWebContents(contents);
     }
     return proceed;
@@ skipping 16 matching lines @@
     return false;
   }
 
   return true;
 }
 
 bool FastUnloadController::ShouldCloseWindow() {
   if (HasCompletedUnloadProcessing())
     return true;
 
+  if (browser_->is_devtools() &&
+      DevToolsWindow::ShouldCloseDevToolsBrowser(browser_)) {
+    return true;
+  }
+
   // The behavior followed here varies based on the current phase of the
   // operation and whether a batched shutdown is in progress.
   //
   // If there are tabs with outstanding beforeunload handlers:
   // 1. If a batched shutdown is in progress: return false.
   //    This is to prevent interference with batched shutdown already in
   //    progress.
   // 2. Otherwise: start sending beforeunload events and return false.
   //
   // Otherwise, If there are no tabs with outstanding beforeunload handlers:
   // 3. If a batched shutdown is in progress: start sending unload events and
   //    return false.
   // 4. Otherwise: return true.
   is_attempting_to_close_browser_ = true;
   // Cases 1 and 4.
   bool need_beforeunload_fired = TabsNeedBeforeUnloadFired();
   if (need_beforeunload_fired == is_calling_before_unload_handlers())
     return !need_beforeunload_fired;
 
   // Cases 2 and 3.
   on_close_confirmed_.Reset();
   ProcessPendingTabs();
   return false;
 }
 
 bool FastUnloadController::CallBeforeUnloadHandlers(
     const base::Callback<void(bool)>& on_close_confirmed) {
-  if (!TabsNeedBeforeUnloadFired())
+  // DevTools browser will get its beforeunload events triggered on
+  // inspected tab closing

[jeremy, 2013/11/07 14:18:34]

will get -> gets

[lushnikov, 2013/11/07 17:18:10]

Done.

+  if (browser_->is_devtools() || !TabsNeedBeforeUnloadFired())
     return false;
 
   on_close_confirmed_ = on_close_confirmed;
   is_attempting_to_close_browser_ = true;
   ProcessPendingTabs();
   return true;
 }
 
 void FastUnloadController::ResetBeforeUnloadHandlers() {
   if (!is_calling_before_unload_handlers())
     return;
   CancelWindowClose();
 }
 
 bool FastUnloadController::TabsNeedBeforeUnloadFired() {
   if (!tabs_needing_before_unload_.empty() ||
       tab_needing_before_unload_ack_ != NULL)
     return true;
 
   if (!is_calling_before_unload_handlers() && !tabs_needing_unload_.empty())
     return false;
 
   for (int i = 0; i < browser_->tab_strip_model()->count(); ++i) {
     content::WebContents* contents =
         browser_->tab_strip_model()->GetWebContentsAt(i);
     if (!ContainsKey(tabs_needing_unload_, contents) &&
         !ContainsKey(tabs_needing_unload_ack_, contents) &&
         tab_needing_before_unload_ack_ != contents &&
-        contents->NeedToFireBeforeUnload())
+        (contents->NeedToFireBeforeUnload() ||
+        DevToolsWindow::NeedToFireBeforeUnload(contents)))

[jeremy, 2013/11/07 14:18:34]

This expression is already kind of crazy - can we split this out into a boolean
and then just check that boolean inside the if?

[lushnikov, 2013/11/07 17:18:10]

Done.

       tabs_needing_before_unload_.insert(contents);
   }
   return !tabs_needing_before_unload_.empty();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // FastUnloadController, content::NotificationObserver implementation:
 
 void FastUnloadController::Observe(
       int type,
@@ skipping 111 matching lines @@
     content::WebContents* contents = *it;
     tabs_needing_before_unload_.erase(it);
     // Null check render_view_host here as this gets called on a PostTask and
     // the tab's render_view_host may have been nulled out.
     if (contents->GetRenderViewHost()) {
       tab_needing_before_unload_ack_ = contents;
 
       CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(contents);
       core_tab_helper->OnCloseStarted();
 
-      contents->GetRenderViewHost()->FirePageBeforeUnload(false);
+      if (!DevToolsWindow::InterceptPageBeforeUnload(contents))

[jeremy, 2013/11/07 14:18:34]

I don't really think this method name conveys what is happening, but I don't
have any ideas for a better name, sorry :(

This needs a comment though, something like:

First let Devtools put up a warning dialog if needed, then give the renderview a
chance to put up a dialog.  See ... for details.

[lushnikov, 2013/11/07 17:18:10]

Done.

+        contents->GetRenderViewHost()->FirePageBeforeUnload(false);
     } else {
       ProcessPendingTabs();
     }
     return;
   }
 
   if (is_calling_before_unload_handlers()) {
     on_close_confirmed_.Run(true);
     return;
   }
-
   // Process all the unload handlers. (The beforeunload handlers have finished.)
   if (!tabs_needing_unload_.empty()) {
     browser_->OnWindowClosing();
 
     // Run unload handlers detached since no more interaction is possible.
     WebContentsSet::iterator it = tabs_needing_unload_.begin();
     while (it != tabs_needing_unload_.end()) {
       WebContentsSet::iterator current = it++;
       content::WebContents* contents = *current;
       tabs_needing_unload_.erase(current);
@@ skipping 37 matching lines @@
       tab_needing_before_unload_ack_ == NULL &&
       tabs_needing_unload_.empty() &&
       tabs_needing_unload_ack_.empty();
 }
 
 void FastUnloadController::CancelWindowClose() {
   // Closing of window can be canceled from a beforeunload handler.
   DCHECK(is_attempting_to_close_browser_);
   tabs_needing_before_unload_.clear();
   if (tab_needing_before_unload_ack_ != NULL) {
-
     CoreTabHelper* core_tab_helper =
         CoreTabHelper::FromWebContents(tab_needing_before_unload_ack_);
     core_tab_helper->OnCloseCanceled();
+    DevToolsWindow::PageClosingCanceled(tab_needing_before_unload_ack_);

[jeremy, 2013/11/07 14:18:34]

*OnPageCloseCanceled() ?

[lushnikov, 2013/11/07 17:18:10]

Done.

     tab_needing_before_unload_ack_ = NULL;
   }
   for (WebContentsSet::iterator it = tabs_needing_unload_.begin();
        it != tabs_needing_unload_.end(); it++) {
     content::WebContents* contents = *it;
 
     CoreTabHelper* core_tab_helper = CoreTabHelper::FromWebContents(contents);
     core_tab_helper->OnCloseCanceled();
+    DevToolsWindow::PageClosingCanceled(contents);
   }
   tabs_needing_unload_.clear();
 
   // No need to clear tabs_needing_unload_ack_. Those tabs are already detached.
 
   if (is_calling_before_unload_handlers()) {
     base::Callback<void(bool)> on_close_confirmed = on_close_confirmed_;
     on_close_confirmed_.Reset();
     on_close_confirmed.Run(false);
   }
@@ skipping 30 matching lines @@
 }
 
 void FastUnloadController::PostTaskForProcessPendingTabs() {
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&FastUnloadController::ProcessPendingTabs,
                  weak_factory_.GetWeakPtr()));
 }
 
 }  // namespace chrome