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

chrome/browser/devtools/devtools_window.h

Index: chrome/browser/devtools/devtools_window.h
diff --git a/chrome/browser/devtools/devtools_window.h b/chrome/browser/devtools/devtools_window.h
index 32c3d71b26bd23872dd77c971c13ab0efffab4ad..da9e8c130ca21e6762198aa01597a2b503efd6f1 100644
--- a/chrome/browser/devtools/devtools_window.h
+++ b/chrome/browser/devtools/devtools_window.h
@@ -130,6 +130,81 @@ class DevToolsWindow : private content::NotificationObserver,
 
   void Show(const DevToolsToggleAction& action);
 
+  // BeforeUnload interception ////////////////////////////////////////////////
+
+  // In order to preserve any edits the user may have made in DevTools, the
+  // beforeunload event of the inspected page is hooked - DevTools gets the
+  // first shot at handling beforeunload and presents a dialog to the user. If
+  // the user accepts the dialog then the script is given a chance to handle
+  // it. This way 2 dialogs may be displayed: one from the DevTools asking the
+  // user to confirm that they're ok with their edits going away and another

[jeremy, 2013/11/13 10:52:34]

*devtools edits

[lushnikov, 2013/11/13 13:39:08]

Done.

+  // from the webpage as the result of its beforeunload handler.
+  // The following set of methods handle beforeunload event flow through
+  // DevTools window. When the |contents| with DevTools opened on them are
+  // getting closed, the following sequence of calls takes place:
+  // 1. Instead of firing beforeunload event on |contents|,
+  //    method |DevToolsWindow::InterceptPageBeforeUnload| gets called.

[jeremy, 2013/11/13 10:52:34]

How about something like:
1. |DevToolsWindow::InterceptPageBeforeUnload| is called by |contents| and
indicates whether DevTools needs to intercept the beforeunload event.

If InterceptPageBeforeUnload() returns true then:
<If it returns false, I assume none of the below functions get called? Even
OnPageCloseCancelled?>

[lushnikov, 2013/11/13 13:39:08]

Reworded and put smth like this

+  // 2. |DevToolsWindow::InterceptPageBeforeUnload| fires beforeunload event
+  //    for DevTools frontend, which will asynchronously call
+  //    |WebContentsDelegate::BeforeUnloadFired| method with the boolean
+  //    argument describing user decision if he wants to close DevTools.

[jeremy, 2013/11/13 10:52:34]

remove "with the boolean" till end of sentence

[lushnikov, 2013/11/13 13:39:08]

Done.

+  //    In case of docked DevTools window, DevTools are set as a delegate for
+  //    its frontend, so method |DevToolsWindow::BeforeUnloadFired| will be
+  //    called. But if DevTools window is undocked, then method
+  //    |Browser::BeforeUnloadFired| will be called, and then with the help of
+  //    proxying |DevToolsWindow::HandleBeforeUnload| method will end up
+  //    in |DevToolsWindow::BeforeUnloadFired| as well.

[jeremy, 2013/11/13 10:52:34]

For the rest of this, how about something like:
If Devtools is docked then it's set as the window delegate so
|DevToolsWindow::BeforeUnloadFired is called directly|.

If DevToos is undocked it's not set as the delegate then the call to
BeforeUnloadFired is proxied through HandleBeforeUnload() rather than getting
called directly.

[lushnikov, 2013/11/13 13:39:08]

Done.

+  // 3a. If |DevToolsWindow::BeforeUnloadFired| was called with |proceed|
+  //     argument set to false, then it calls method
+  //     |WebContentsDelegate::BeforeUnloadFired| on |contents|
+  //     delegate with the |proceed| argument set to false. This looks as if
+  //     |contents| got its beforeunload event fired and user rejected page
+  //     close.

[jeremy, 2013/11/13 10:52:34]

If |DevToolsWindow::BeforeUnloadFired| is called with proceed=false it calls
through to the content's BeforeUnloadFired() dialog, which from the WebContents
perspective looks the same as the Content's own beforeunload dialog having had
it's 'cancel' button clicked.

[lushnikov, 2013/11/13 13:39:08]

Done.

+  // 3b. If |DevToolsWindow::BeforeUnloadFired| was called with |proceed|
+  //     argument set to true, then it fires beforeunload event on |contents|
+  //     and everything proceeds as it normally would without the Devtools
+  //     interception.
+  // 4. If the user cancels the dialog put up by either the WebContents or
+  //    DevTools frontend, then |contents|'s |BeforeUnloadFired| callback is
+  //    called with the proceed argument set to false, this causes
+  //    |DevToolsWindow::OnPageCloseCancelled| to be called.
+
+  // DevTools window in undocked state is not set as a delegate of
+  // its frontend. Instead, an instance of browser is set as the delegate, and
+  // thus beforeunload event callback from DevTools frontend is not delivered
+  // to the instance of DevTools window, which is solely responsible for
+  // managing custom beforeunload event flow.
+  // This is a helper method to route callback from
+  // |Browser::BeforeUnloadFired| back to |DevToolsWindow::BeforeUnloadFired|.
+  // The |proceed| bool tells us if the user chose to proceed closing the
+  // |contents|, and out parameter |proceed_to_fire_unload| signals whether the
+  // method client should continue to fire unload event.
+  // Returns true if DevTools window is in a state of intercepting beforeunload
+  // event and if it will manage unload process on its own.

[jeremy, 2013/11/13 10:52:34]

* |proceed| - true if the user clicked 'ok' in the beforeunload dialog, false
otherwise.
* |proceed_to_fire_unload| - On output, whether we should continue to fire the
unload event or stop things here.

[lushnikov, 2013/11/13 13:39:08]

Done.

+  static bool HandleBeforeUnload(content::WebContents* contents,
+                                 bool proceed,
+                                 bool* proceed_to_fire_unload);
+
+  // This function returns true if there is a DevTools window inspecting
+  // given |contents| which would like to intercept beforeunload events as
+  // described above. In this case clients should not fire beforeunload
+  // event on |contents| themselfes as DevTools window will take care of it.

[jeremy, 2013/11/13 10:52:34]

//  Sends a beforeunload event to the Devtools frontend if needed, returns true
if the content's event was intercepted by devtools false if not and the caller
should continue handling the event as usual.

[lushnikov, 2013/11/13 13:39:08]

Done.

+  static bool InterceptPageBeforeUnload(content::WebContents* contents);
+
+  // Returns true if DevTools browser has already fired its beforeunload event
+  // as a result of beforeunload event interception.
+  static bool HasFiredBeforeUnloadEventForDevToolsBrowser(Browser* browser);
+
+  // Returns true if DevTools window would like to hook beforeunload event
+  // of this |contents|.
+  static bool NeedsToInterceptBeforeUnload(content::WebContents* contents);
+
+  // Notify DevTools window that closing of |contents| was cancelled
+  // by user.
+  static void OnPageCloseCanceled(content::WebContents* contents);
+
+  void SetDockSideForTest(DevToolsDockSide dock_side);
+
  private:
   friend class DevToolsControllerTest;
 
@@ -281,6 +356,7 @@ class DevToolsWindow : private content::NotificationObserver,
   int width_;
   int height_;
   DevToolsDockSide dock_side_before_minimized_;
+  bool inspected_page_is_closing_;
 
   scoped_ptr<DevToolsEmbedderMessageDispatcher> embedder_message_dispatcher_;
   base::WeakPtrFactory<DevToolsWindow> weak_factory_;