Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1045)

Unified Diff: chrome/browser/devtools/devtools_window.h

Issue 23835007: DevTools: Do not close devtools if there are dirty files in workspace (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address Jeremy's comments Created 7 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..1e7895dd90c5d8ae42300681c106079283921760 100644
--- a/chrome/browser/devtools/devtools_window.h
+++ b/chrome/browser/devtools/devtools_window.h
@@ -130,6 +130,82 @@ 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 devtools edits going away and
jeremy 2013/11/14 08:53:13 nit: devtools -> DevTools
lushnikov 2013/11/14 09:49:39 Converted everything to "devtools" instead as this
+ // another 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. |DevToolsWindow::InterceptPageBeforeUnload| is called and indicates
+ // whether DevTools intercept the beforeunload event.
+ // If InterceptPageBeforeUnload() returns true then the following steps
+ // will take place; otherwise only step 4 will be reached.
jeremy 2013/11/14 08:53:13 reached -> reached and none of the corresponding f
lushnikov 2013/11/14 09:49:39 Done.
+ // 2. |DevToolsWindow::InterceptPageBeforeUnload| fires beforeunload event
+ // for DevTools frontend, which will asynchronously call
+ // |WebContentsDelegate::BeforeUnloadFired| method.
+ // In case of docked DevTools window, DevTools are set as a delegate for
+ // its frontend, so method |DevToolsWindow::BeforeUnloadFired| will be
+ // called directly.
+ // If DevTools window is undocked it's not set as the delegate so the call
+ // to BeforeUnloadFired is proxied through HandleBeforeUnload() rather
+ // than getting called directly.
+ // 3a. If |DevToolsWindow::BeforeUnloadFired| is called with |proceed|=false
+ // it calls throught to the content's BeforeUnloadFired(), which from the
+ // WebContents perspective looks the same as the |content|'s own
+ // beforeunload dialog having had it's 'stay on this page' button clicked.
+ // 3b. If |DevToolsWindow::BeforeUnloadFired| was called with |proceed|
jeremy 2013/11/14 08:53:13 Replace everything up to ',' with: If |proceed| =
lushnikov 2013/11/14 09:49:39 Done.
+ // 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|.
+ // * |proceed| - true if the user clicked 'ok' in the beforeunload dialog,
+ // false otherwise.
+ // * |proceed_to_fire_unload| - output parameter, whether we should continue
+ // to fire the unload event or stop things here.
+ // Returns true if DevTools window is in a state of intercepting beforeunload
+ // event and if it will manage unload process on its own.
+ static bool HandleBeforeUnload(content::WebContents* contents,
+ bool proceed,
+ bool* proceed_to_fire_unload);
+
+ // Returns true if this contents beforeunload event was intercepted by
+ // DevTools and false otherwise. If the event was intercepted, caller should
+ // not fire beforeunlaod event on |contents| itself as DevTools window will
+ // take care of it, otherwise caller should continue handling the event as
+ // usual.
+ 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 +357,7 @@ class DevToolsWindow : private content::NotificationObserver,
int width_;
int height_;
DevToolsDockSide dock_side_before_minimized_;
+ bool inspected_page_is_closing_;
jeremy 2013/11/14 08:53:13 Can you add a comment explaining what this does?
lushnikov 2013/11/14 09:49:39 Done.
scoped_ptr<DevToolsEmbedderMessageDispatcher> embedder_message_dispatcher_;
base::WeakPtrFactory<DevToolsWindow> weak_factory_;

Powered by Google App Engine
This is Rietveld 408576698