[DevTools] Remove dock side knowledge from browser.

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_reader.h"
@@ skipping 210 matching lines @@
 }
 
 // DevToolsWindow -------------------------------------------------------------
 
 namespace {
 
 typedef std::vector<DevToolsWindow*> DevToolsWindows;
 base::LazyInstance<DevToolsWindows>::Leaky g_instances =
     LAZY_INSTANCE_INITIALIZER;
 
-const char kPrefBottom[] = "dock_bottom";
-const char kPrefRight[] = "dock_right";
-const char kPrefUndocked[] = "undocked";
-
-const char kDockSideBottom[] = "bottom";
-const char kDockSideRight[] = "right";
+// TODO(dgozman): remove after switching to SetIsDocked.
 const char kDockSideUndocked[] = "undocked";
-const char kDockSideMinimized[] = "minimized";
 
 static const char kFrontendHostId[] = "id";
 static const char kFrontendHostMethod[] = "method";
 static const char kFrontendHostParams[] = "params";
 
 std::string SkColorToRGBAString(SkColor color) {
   // We avoid StringPrintf because it will use locale specific formatters for
   // the double (e.g. ',' instead of '.' in German).
   return "rgba(" + base::IntToString(SkColorGetR(color)) + "," +
       base::IntToString(SkColorGetG(color)) + "," +
@@ skipping 34 matching lines @@
 
 // static
 std::string DevToolsWindow::GetDevToolsWindowPlacementPrefKey() {
   return std::string(prefs::kBrowserWindowPlacement) + "_" +
       std::string(kDevToolsApp);
 }
 
 // static
 void DevToolsWindow::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
-  registry->RegisterBooleanPref(
-      prefs::kDevToolsOpenDocked, true,
-      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
-  registry->RegisterStringPref(
-      prefs::kDevToolsDockSide, kDockSideBottom,
-      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterDictionaryPref(
       prefs::kDevToolsEditedFiles,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterDictionaryPref(
       prefs::kDevToolsFileSystemPaths,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterStringPref(
       prefs::kDevToolsAdbKey, std::string(),
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 
@@ skipping 16 matching lines @@
   registry->RegisterDictionaryPref(
       prefs::kDevToolsPortForwardingConfig,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 // static
 DevToolsWindow* DevToolsWindow::GetDockedInstanceForInspectedTab(
     content::WebContents* inspected_web_contents) {
   DevToolsWindow* window = GetInstanceForInspectedRenderViewHost(
       inspected_web_contents->GetRenderViewHost());
-  return (window && window->IsDocked()) ? window : NULL;
+  return (window && window->is_docked_) ? window : NULL;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::GetInstanceForInspectedRenderViewHost(
       content::RenderViewHost* inspected_rvh) {
   if (!inspected_rvh || !DevToolsAgentHost::HasFor(inspected_rvh))
     return NULL;
 
   scoped_refptr<DevToolsAgentHost> agent(DevToolsAgentHost::GetOrCreateFor(
       inspected_rvh));
   return FindDevToolsWindow(agent.get());
 }
 
 // static
 bool DevToolsWindow::IsDevToolsWindow(content::RenderViewHost* window_rvh) {
   return AsDevToolsWindow(window_rvh) != NULL;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::OpenDevToolsWindowForWorker(
     Profile* profile,
     DevToolsAgentHost* worker_agent) {
   DevToolsWindow* window = FindDevToolsWindow(worker_agent);
   if (!window) {
     window = DevToolsWindow::CreateDevToolsWindowForWorker(profile);
     // Will disconnect the current client host if there is one.
     content::DevToolsManager::GetInstance()->RegisterDevToolsClientHostFor(
         worker_agent, window->frontend_host_.get());
   }
-  window->Show(DevToolsToggleAction::Show());
+  window->ScheduleShow(DevToolsToggleAction::Show());
   return window;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
     Profile* profile) {
   content::RecordAction(content::UserMetricsAction("DevTools_InspectWorker"));
-  return Create(profile, GURL(), NULL, DEVTOOLS_DOCK_SIDE_UNDOCKED, true,
-                false, false);
+  return Create(profile, GURL(), NULL, true, false, false);
 }
 
 // static
 DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
     content::RenderViewHost* inspected_rvh) {
   return ToggleDevToolsWindow(
       inspected_rvh, true, DevToolsToggleAction::Show());
 }
 
 // static
+DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
+    content::RenderViewHost* inspected_rvh,
+    const DevToolsToggleAction& action) {
+  return ToggleDevToolsWindow(
+      inspected_rvh, true, action);
+}
+
+// static
 DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
     Browser* browser,
     const DevToolsToggleAction& action) {
   if (action.type() == DevToolsToggleAction::kToggle &&
       browser->is_devtools()) {
     browser->tab_strip_model()->CloseAllTabs();
     return NULL;
   }
 
   return ToggleDevToolsWindow(
       browser->tab_strip_model()->GetActiveWebContents()->GetRenderViewHost(),
       action.type() == DevToolsToggleAction::kInspect, action);
 }
 
 // static
 void DevToolsWindow::OpenExternalFrontend(
     Profile* profile,
     const std::string& frontend_url,
     content::DevToolsAgentHost* agent_host) {
   DevToolsWindow* window = FindDevToolsWindow(agent_host);
   if (!window) {
     window = Create(profile, DevToolsUI::GetProxyURL(frontend_url), NULL,
-                    DEVTOOLS_DOCK_SIDE_UNDOCKED, false, true, false);
+                    false, true, false);
     content::DevToolsManager::GetInstance()->RegisterDevToolsClientHostFor(
         agent_host, window->frontend_host_.get());
   }
-  window->Show(DevToolsToggleAction::Show());
+  window->ScheduleShow(DevToolsToggleAction::Show());
 }
 
 // static
 DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
     content::RenderViewHost* inspected_rvh,
     bool force_open,
     const DevToolsToggleAction& action) {
   scoped_refptr<DevToolsAgentHost> agent(
       DevToolsAgentHost::GetOrCreateFor(inspected_rvh));
   content::DevToolsManager* manager = content::DevToolsManager::GetInstance();
   DevToolsWindow* window = FindDevToolsWindow(agent.get());
   bool do_open = force_open;
   if (!window) {
     Profile* profile = Profile::FromBrowserContext(
         inspected_rvh->GetProcess()->GetBrowserContext());
-    DevToolsDockSide dock_side = GetDockSideFromPrefs(profile);
     content::RecordAction(
         content::UserMetricsAction("DevTools_InspectRenderer"));
-    window = Create(profile, GURL(), inspected_rvh, dock_side, false, false,
+    window = Create(profile, GURL(), inspected_rvh, false, false,
         true);
     manager->RegisterDevToolsClientHostFor(agent.get(),
                                            window->frontend_host_.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 minimized, we maximize it.
-  if (window->dock_side_ == DEVTOOLS_DOCK_SIDE_MINIMIZED)
-    window->Restore();
-  else if (!window->IsDocked() || do_open)
-    window->Show(action);
+  // undocked, we show (activate) it.
+  if (!window->is_docked_ || do_open)
+    window->ScheduleShow(action);
   else
     window->CloseWindow();
 
   return window;
 }
 
 // static
 void DevToolsWindow::InspectElement(content::RenderViewHost* inspected_rvh,
                                     int x,
                                     int y) {
@@ skipping 13 matching lines @@
 
 void DevToolsWindow::InspectedContentsClosing() {
   intercepted_page_beforeunload_ = false;
   web_contents_->GetRenderViewHost()->ClosePage();
 }
 
 content::RenderViewHost* DevToolsWindow::GetRenderViewHost() {
   return web_contents_->GetRenderViewHost();
 }
 
-content::DevToolsClientHost* DevToolsWindow::GetDevToolsClientHostForTest() {
-  return frontend_host_.get();
-}
-
 gfx::Insets DevToolsWindow::GetContentsInsets() const {
   return contents_insets_;
 }
 
 gfx::Size DevToolsWindow::GetMinimumSize() const {
   const gfx::Size kMinDevToolsSize = gfx::Size(200, 100);
   return kMinDevToolsSize;
 }
 
+void DevToolsWindow::ScheduleShow(const DevToolsToggleAction& action) {
+  if (load_state_mask_ == kLoadCompleted)
+    Show(action);
+  else
+    action_on_load_ = action;
+}
+
 void DevToolsWindow::Show(const DevToolsToggleAction& action) {
-  if (IsDocked()) {
+  if (action.type() == DevToolsToggleAction::kNothing)

[pfeldman, 2014/01/09 12:52:01]

kNoop

[dgozman, 2014/01/14 15:26:17]

Done.

+    return;
+
+  if (is_docked_) {
+    DCHECK(can_dock_);
     Browser* inspected_browser = NULL;
     int inspected_tab_index = -1;
+    FindInspectedBrowserAndTabIndex(GetInspectedWebContents(),
+                                    &inspected_browser,
+                                    &inspected_tab_index);
+    DCHECK(inspected_browser);
+    DCHECK(inspected_tab_index != -1);
+
     // Tell inspected browser to update splitter and switch to inspected panel.
-    if (!IsInspectedBrowserPopup() &&
-        FindInspectedBrowserAndTabIndex(GetInspectedWebContents(),
-                                        &inspected_browser,
-                                        &inspected_tab_index)) {
-      BrowserWindow* inspected_window = inspected_browser->window();
-      web_contents_->SetDelegate(this);
-      inspected_window->UpdateDevTools();
-      web_contents_->GetView()->SetInitialFocus();
-      inspected_window->Show();
-      TabStripModel* tab_strip_model = inspected_browser->tab_strip_model();
-      tab_strip_model->ActivateTabAt(inspected_tab_index, true);
-      PrefsTabHelper::CreateForWebContents(web_contents_);
-      GetRenderViewHost()->SyncRendererPrefs();
-      ScheduleAction(action);
-      return;
-    }
+    BrowserWindow* inspected_window = inspected_browser->window();
+    web_contents_->SetDelegate(this);
+    inspected_window->UpdateDevTools();
+    web_contents_->GetView()->SetInitialFocus();
+    inspected_window->Show();
 
-    // Sometimes we don't know where to dock. Stay undocked.
-    dock_side_ = DEVTOOLS_DOCK_SIDE_UNDOCKED;
+    TabStripModel* tab_strip_model = inspected_browser->tab_strip_model();
+    tab_strip_model->ActivateTabAt(inspected_tab_index, true);
+    PrefsTabHelper::CreateForWebContents(web_contents_);
+    GetRenderViewHost()->SyncRendererPrefs();
+
+    DoAction(action);
+    return;
   }
 
   // Avoid consecutive window switching if the devtools window has been opened
   // and the Inspect Element shortcut is pressed in the inspected tab.
   bool should_show_window =
       !browser_ || (action.type() != DevToolsToggleAction::kInspect);
 
   if (!browser_)
     CreateDevToolsBrowser();
 
   if (should_show_window) {
     browser_->window()->Show();
     web_contents_->GetView()->SetInitialFocus();
   }
 
-  ScheduleAction(action);
+  DoAction(action);
 }
 
 // static
 bool DevToolsWindow::HandleBeforeUnload(content::WebContents* frontend_contents,
     bool proceed, bool* proceed_to_fire_unload) {
   DevToolsWindow* window = AsDevToolsWindow(
       frontend_contents->GetRenderViewHost());
   if (!window)
     return false;
   if (!window->intercepted_page_beforeunload_)
@@ skipping 51 matching lines @@
   DevToolsWindow *window =
       DevToolsWindow::GetInstanceForInspectedRenderViewHost(
           contents->GetRenderViewHost());
   if (!window)
     return;
   window->intercepted_page_beforeunload_ = false;
   // Propagate to devtools opened on devtools if any.
   DevToolsWindow::OnPageCloseCanceled(window->web_contents());
 }
 
-void DevToolsWindow::SetDockSideForTest(DevToolsDockSide dock_side) {
-  SetDockSide(SideToString(dock_side));
-}
-
 DevToolsWindow::DevToolsWindow(Profile* profile,
                                const GURL& url,
                                content::RenderViewHost* inspected_rvh,
-                               DevToolsDockSide dock_side)
+                               bool can_dock)
     : profile_(profile),
       browser_(NULL),
-      dock_side_(dock_side),
-      is_loaded_(false),
-      action_on_load_(DevToolsToggleAction::Show()),
-      dock_side_before_minimized_(dock_side),
+      is_docked_(false),
+      can_dock_(can_dock),
+      load_state_mask_(can_dock ? kNotLoaded : kIsDockedSet),

[pfeldman, 2014/01/09 12:52:01]

Let add a comment here about supporting for the legacy front-end.

[dgozman, 2014/01/14 15:26:17]

Done.

+      action_on_load_(DevToolsToggleAction::Nothing()),
+      set_is_docked_for_test_on_load_(false),
       intercepted_page_beforeunload_(false),
       weak_factory_(this) {
   web_contents_ =
       content::WebContents::Create(content::WebContents::CreateParams(profile));
   frontend_contents_observer_.reset(new FrontendWebContentsObserver(this));
 
   web_contents_->GetController().LoadURL(url, content::Referrer(),
       content::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
 
   frontend_host_.reset(content::DevToolsClientHost::CreateDevToolsFrontendHost(
@@ skipping 23 matching lines @@
 
   embedder_message_dispatcher_.reset(
       new DevToolsEmbedderMessageDispatcher(this));
 }
 
 // static
 DevToolsWindow* DevToolsWindow::Create(
     Profile* profile,
     const GURL& frontend_url,
     content::RenderViewHost* inspected_rvh,
-    DevToolsDockSide dock_side,
     bool shared_worker_frontend,
     bool external_frontend,
     bool can_dock) {
   if (inspected_rvh) {
     // Check for a place to dock.
     Browser* browser = NULL;
     int tab;
     content::WebContents* inspected_web_contents =
         content::WebContents::FromRenderViewHost(inspected_rvh);
     if (!FindInspectedBrowserAndTabIndex(inspected_web_contents,
                                          &browser, &tab) ||
         browser->is_type_popup()) {
       can_dock = false;
     }
   }
 
   // Create WebContents with devtools.
-  GURL url(GetDevToolsURL(profile, frontend_url, dock_side,
+  GURL url(GetDevToolsURL(profile, frontend_url,
                           shared_worker_frontend,
                           external_frontend,
                           can_dock));
-  return new DevToolsWindow(profile, url, inspected_rvh, dock_side);
+  return new DevToolsWindow(profile, url, inspected_rvh, can_dock);
 }
 
 // static
 GURL DevToolsWindow::GetDevToolsURL(Profile* profile,
                                     const GURL& base_url,
-                                    DevToolsDockSide dock_side,
                                     bool shared_worker_frontend,
                                     bool external_frontend,
                                     bool can_dock) {
   if (base_url.SchemeIs("data"))
     return base_url;
 
   std::string frontend_url(
       base_url.is_empty() ? chrome::kChromeUIDevToolsURL : base_url.spec());
   ThemeService* tp = ThemeServiceFactory::GetForProfile(profile);
   DCHECK(tp);
   std::string url_string(
       frontend_url +
       ((frontend_url.find("?") == std::string::npos) ? "?" : "&") +
-      "dockSide=" + SideToString(dock_side) +
+      "dockSide=undocked" + // TODO(dgozman): remove this support in M38.
       "&toolbarColor=" +
       SkColorToRGBAString(tp->GetColor(ThemeProperties::COLOR_TOOLBAR)) +
       "&textColor=" +
       SkColorToRGBAString(tp->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT)));
   if (shared_worker_frontend)
     url_string += "&isSharedWorker=true";
   if (external_frontend)
     url_string += "&remoteFrontend=true";
   if (can_dock)
     url_string += "&can_dock=true";
@@ skipping 27 matching lines @@
   DevToolsWindows* instances = &g_instances.Get();
   for (DevToolsWindows::iterator it(instances->begin()); it != instances->end();
        ++it) {
     if ((*it)->web_contents_->GetRenderViewHost() == window_rvh)
       return *it;
   }
   return NULL;
 }
 
 // static
-DevToolsDockSide DevToolsWindow::GetDockSideFromPrefs(Profile* profile) {
-  std::string dock_side =
-      profile->GetPrefs()->GetString(prefs::kDevToolsDockSide);
-
-  // Migrate prefs.
-  const char kOldPrefBottom[] = "bottom";
-  const char kOldPrefRight[] = "right";
-  if ((dock_side == kOldPrefBottom) || (dock_side == kOldPrefRight)) {
-    if (!profile->GetPrefs()->GetBoolean(prefs::kDevToolsOpenDocked))
-      return DEVTOOLS_DOCK_SIDE_UNDOCKED;
-    return (dock_side == kOldPrefBottom) ?
-        DEVTOOLS_DOCK_SIDE_BOTTOM : DEVTOOLS_DOCK_SIDE_RIGHT;
-  }
-
-  if (dock_side == kPrefUndocked)
-    return DEVTOOLS_DOCK_SIDE_UNDOCKED;
-  if (dock_side == kPrefRight)
-    return DEVTOOLS_DOCK_SIDE_RIGHT;
-  // Default to docked to bottom.
-  return DEVTOOLS_DOCK_SIDE_BOTTOM;
-}
-
-// static
-std::string DevToolsWindow::SideToString(DevToolsDockSide dock_side) {
-  switch (dock_side) {
-    case DEVTOOLS_DOCK_SIDE_UNDOCKED:  return kDockSideUndocked;
-    case DEVTOOLS_DOCK_SIDE_RIGHT:     return kDockSideRight;
-    case DEVTOOLS_DOCK_SIDE_BOTTOM:    return kDockSideBottom;
-    case DEVTOOLS_DOCK_SIDE_MINIMIZED: return kDockSideMinimized;
-    default:                           return kDockSideUndocked;
-  }
-}
-
-// static
-DevToolsDockSide DevToolsWindow::SideFromString(
-    const std::string& dock_side) {
-  if (dock_side == kDockSideRight)
-    return DEVTOOLS_DOCK_SIDE_RIGHT;
-  if (dock_side == kDockSideBottom)
-    return DEVTOOLS_DOCK_SIDE_BOTTOM;
-  return (dock_side == kDockSideMinimized) ?
-      DEVTOOLS_DOCK_SIDE_MINIMIZED : DEVTOOLS_DOCK_SIDE_UNDOCKED;
+bool DevToolsWindow::IsDockedFromString(const std::string& dock_side) {
+  return dock_side != kDockSideUndocked;
 }
 
 void DevToolsWindow::Observe(int type,
                              const content::NotificationSource& source,
                              const content::NotificationDetails& details) {
   DCHECK_EQ(chrome::NOTIFICATION_BROWSER_THEME_CHANGED, type);
   UpdateTheme();
 }
 
 content::WebContents* DevToolsWindow::OpenURLFromTab(
@@ skipping 32 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) {
-  CHECK(IsDocked());
+  CHECK(is_docked_);
   // 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.
   // Embedding DevTools window will be deleted as a result of
   // WebContentsDestroyed callback.
   delete web_contents_;
 }
 
@@ skipping 19 matching lines @@
       DCHECK(!should_proceed);
     }
     *proceed_to_fire_unload = false;
   }
 }
 
 bool DevToolsWindow::PreHandleKeyboardEvent(
     content::WebContents* source,
     const content::NativeWebKeyboardEvent& event,
     bool* is_keyboard_shortcut) {
-  if (IsDocked()) {
+  if (is_docked_) {
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
     if (inspected_window) {
       return inspected_window->PreHandleKeyboardEvent(event,
                                                       is_keyboard_shortcut);
     }
   }
   return false;
 }
 
 void DevToolsWindow::HandleKeyboardEvent(
     content::WebContents* source,
     const content::NativeWebKeyboardEvent& event) {
-  if (IsDocked()) {
+  if (is_docked_) {
     if (event.windowsKeyCode == 0x08) {
       // Do not navigate back in history on Windows (http://crbug.com/74156).
       return;
     }
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
     if (inspected_window)
       inspected_window->HandleKeyboardEvent(event);
   }
 }
 
@@ skipping 12 matching lines @@
 }
 
 void DevToolsWindow::RunFileChooser(content::WebContents* web_contents,
                                     const content::FileChooserParams& params) {
   FileSelectHelper::RunFileChooser(web_contents, params);
 }
 
 void DevToolsWindow::WebContentsFocused(content::WebContents* contents) {
   Browser* inspected_browser = NULL;
   int inspected_tab_index = -1;
-  if (IsDocked() && FindInspectedBrowserAndTabIndex(GetInspectedWebContents(),
+  if (is_docked_ && FindInspectedBrowserAndTabIndex(GetInspectedWebContents(),
                                                     &inspected_browser,
                                                     &inspected_tab_index))
     inspected_browser->window()->WebContentsFocused(contents);
 }
 
 void DevToolsWindow::DispatchOnEmbedder(const std::string& message) {
   std::string method;
   base::ListValue empty_params;
   base::ListValue* params = &empty_params;
 
@@ skipping 14 matching lines @@
   std::string error = embedder_message_dispatcher_->Dispatch(method, params);
   if (id) {
     scoped_ptr<base::Value> id_value(base::Value::CreateIntegerValue(id));
     scoped_ptr<base::Value> error_value(base::Value::CreateStringValue(error));
     CallClientFunction("InspectorFrontendAPI.embedderMessageAck",
                        id_value.get(), error_value.get(), NULL);
   }
 }
 
 void DevToolsWindow::ActivateWindow() {
-  if (IsDocked() && GetInspectedBrowserWindow())
+  if (is_docked_ && GetInspectedBrowserWindow())
     web_contents_->GetView()->Focus();
-  else if (!IsDocked() && !browser_->window()->IsActive())
+  else if (!is_docked_ && !browser_->window()->IsActive())
     browser_->window()->Activate();
 }
 
 void DevToolsWindow::ActivateContents(content::WebContents* contents) {
-  if (IsDocked()) {
+  if (is_docked_) {
     content::WebContents* inspected_tab = this->GetInspectedWebContents();
     inspected_tab->GetDelegate()->ActivateContents(inspected_tab);
   } else {
     browser_->window()->Activate();
   }
 }
 
 void DevToolsWindow::CloseWindow() {
-  DCHECK(IsDocked());
+  DCHECK(is_docked_);
   web_contents_->GetRenderViewHost()->FirePageBeforeUnload(false);

[pfeldman, 2014/01/09 12:52:01]

Lets reset load_state_mask_ to kClosing here.

[dgozman, 2014/01/14 15:26:17]

I've set action_on_load to NoOp instead.

 }

[pfeldman, 2014/01/09 13:02:03]

if load_state_mask_ != IsLoaded, you should call
HandleBeforeUnload(web_contents_, true, foo);

[dgozman, 2014/01/14 15:26:17]

After discussing with lushnikov@, I've added a check to
InterceptPageBeforeUnload.

 
 void DevToolsWindow::SetContentsInsets(
     int top, int left, int bottom, int right) {
   if (contents_insets_.top() == top &&
       contents_insets_.left() == left &&
       contents_insets_.bottom() == bottom &&
       contents_insets_.right() == right) {
     return;
   }
 
   contents_insets_ = gfx::Insets(top, left, bottom, right);
-  if (IsDocked()) {
+  if (is_docked_) {
     // Update inspected window.
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
     if (inspected_window)
       inspected_window->UpdateDevTools();
   }
 }
 
 void DevToolsWindow::MoveWindow(int x, int y) {
-  if (!IsDocked()) {
+  if (!is_docked_) {
     gfx::Rect bounds = browser_->window()->GetBounds();
     bounds.Offset(x, y);
     browser_->window()->SetBounds(bounds);
   }
 }
 
+void DevToolsWindow::SetIsDockedAndShowImmediatelyForTest(bool is_docked) {
+  DCHECK(!is_docked || can_dock_);
+  if (load_state_mask_ == kLoadCompleted) {
+    SetIsDocked(is_docked);
+    base::FundamentalValue docked(is_docked_);
+    CallClientFunction("InspectorFrontendAPI.setIsDocked", &docked, NULL, NULL);

[pfeldman, 2014/01/09 12:52:01]

We don't need this as long as existing tests pass.

[dgozman, 2014/01/14 15:26:17]

Done.

+  } else {
+    is_docked_ = is_docked;
+    Show(DevToolsToggleAction::Show());
+    set_is_docked_for_test_on_load_ = true;
+    load_state_mask_ |= kIsDockedSet;
+    if (load_state_mask_ == kLoadCompleted)
+      LoadCompleted();
+  }
+}
+
 void DevToolsWindow::SetDockSide(const std::string& side) {
-  DevToolsDockSide requested_side = SideFromString(side);
-  bool dock_requested = requested_side != DEVTOOLS_DOCK_SIDE_UNDOCKED;
-  bool is_docked = IsDocked();
+  // TODO(dgozman): remove this method after frontend switches to SetIsDocked.
+  SetIsDocked(IsDockedFromString(side));
+}
 
-  if (dock_requested &&
-      (!GetInspectedWebContents() || !GetInspectedBrowserWindow() ||
-       IsInspectedBrowserPopup())) {
-      // Cannot dock, avoid window flashing due to close-reopen cycle.
+void DevToolsWindow::SetIsDocked(bool dock_requested) {
+  if (dock_requested && !can_dock_)

[pfeldman, 2014/01/09 12:52:01]

Lets add DCHECK and proceed as if dock_requested == false.

[dgozman, 2014/01/14 15:26:17]

Done.

+    return;
+
+  bool was_docked = is_docked_;
+  is_docked_ = dock_requested;
+
+  if (load_state_mask_ != kLoadCompleted) {
+    load_state_mask_ |= kIsDockedSet;
+    if (load_state_mask_ == kLoadCompleted)
+      LoadCompleted();
     return;
   }
 
-  if ((dock_side_ != DEVTOOLS_DOCK_SIDE_MINIMIZED) &&
-      (requested_side == DEVTOOLS_DOCK_SIDE_MINIMIZED))
-    dock_side_before_minimized_ = dock_side_;
+  if (dock_requested == was_docked)
+    return;
 
-  dock_side_ = requested_side;
-  if (dock_requested && !is_docked) {
+  if (dock_requested && !was_docked) {
     // Detach window from the external devtools browser. It will lead to
     // the browser object's close and delete. Remove observer first.
     TabStripModel* tab_strip_model = browser_->tab_strip_model();
     tab_strip_model->DetachWebContentsAt(
         tab_strip_model->GetIndexOfWebContents(web_contents_));
     browser_ = NULL;
-  } else if (!dock_requested && is_docked) {
+  } else if (!dock_requested && was_docked) {
     // Update inspected window to hide split and reset it.
     BrowserWindow* inspected_window = GetInspectedBrowserWindow();
     if (inspected_window)
       inspected_window->UpdateDevTools();
   }
 
-  if (dock_side_ != DEVTOOLS_DOCK_SIDE_MINIMIZED) {
-    std::string pref_value = kPrefBottom;
-    switch (dock_side_) {
-      case DEVTOOLS_DOCK_SIDE_UNDOCKED:
-          pref_value = kPrefUndocked;
-          break;
-      case DEVTOOLS_DOCK_SIDE_RIGHT:
-          pref_value = kPrefRight;
-          break;
-      case DEVTOOLS_DOCK_SIDE_BOTTOM:
-          pref_value = kPrefBottom;
-          break;
-      case DEVTOOLS_DOCK_SIDE_MINIMIZED:
-          // We don't persist minimized state.
-          break;
-    }
-    profile_->GetPrefs()->SetString(prefs::kDevToolsDockSide, pref_value);
-  }
-
   Show(DevToolsToggleAction::Show());
 }
 
 void DevToolsWindow::OpenInNewTab(const std::string& url) {
   content::OpenURLParams params(
       GURL(url), content::Referrer(), NEW_FOREGROUND_TAB,
       content::PAGE_TRANSITION_LINK, false);
   content::WebContents* inspected_web_contents = GetInspectedWebContents();
   if (inspected_web_contents) {
     inspected_web_contents->OpenURL(params);
@@ skipping 205 matching lines @@
   base::FundamentalValue request_id_value(request_id);
   base::StringValue file_system_path_value(file_system_path);
   CallClientFunction("InspectorFrontendAPI.searchCompleted", &request_id_value,
                      &file_system_path_value, &file_paths_value);
 }
 
 void DevToolsWindow::ShowDevToolsConfirmInfoBar(
     const base::string16& message,
     const InfoBarCallback& callback) {
   DevToolsConfirmInfoBarDelegate::Create(
-      IsDocked() ?
+      is_docked_ ?
           InfoBarService::FromWebContents(GetInspectedWebContents()) :
           InfoBarService::FromWebContents(web_contents_),
       callback, message);
 }
 
 void DevToolsWindow::CreateDevToolsBrowser() {
   std::string wp_key = GetDevToolsWindowPlacementPrefKey();
   PrefService* prefs = profile_->GetPrefs();
   const base::DictionaryValue* wp_pref = prefs->GetDictionary(wp_key.c_str());
   if (!wp_pref || wp_pref->empty()) {
@@ skipping 36 matching lines @@
 }
 
 BrowserWindow* DevToolsWindow::GetInspectedBrowserWindow() {
   Browser* browser = NULL;
   int tab;
   return FindInspectedBrowserAndTabIndex(GetInspectedWebContents(),
                                          &browser, &tab) ?
       browser->window() : NULL;
 }
 
-bool DevToolsWindow::IsInspectedBrowserPopup() {
-  Browser* browser = NULL;
-  int tab;
-  return FindInspectedBrowserAndTabIndex(GetInspectedWebContents(),
-                                         &browser, &tab) &&
-      browser->is_type_popup();
-}
-
-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::ScheduleAction(const DevToolsToggleAction& action) {
-  action_on_load_ = action;
-  if (is_loaded_)
-    DoAction();
-}
-
-void DevToolsWindow::DoAction() {
-  UpdateFrontendDockSide();
-  switch (action_on_load_.type()) {
+void DevToolsWindow::DoAction(const DevToolsToggleAction& action) {
+  switch (action.type()) {
     case DevToolsToggleAction::kShowConsole:
       CallClientFunction("InspectorFrontendAPI.showConsole", NULL, NULL, NULL);
       break;
 
     case DevToolsToggleAction::kInspect:
       CallClientFunction("InspectorFrontendAPI.enterInspectElementMode", NULL,
                          NULL, NULL);
       break;
 
     case DevToolsToggleAction::kShow:
     case DevToolsToggleAction::kToggle:
       // Do nothing.
       break;
 
     case DevToolsToggleAction::kReveal: {
       const DevToolsToggleAction::RevealParams* params =
-          action_on_load_.params();
+          action.params();
       CHECK(params);
       base::StringValue url_value(params->url);
       base::FundamentalValue line_value(static_cast<int>(params->line_number));
       base::FundamentalValue column_value(
           static_cast<int>(params->column_number));
       CallClientFunction("InspectorFrontendAPI.revealSourceLine",
                          &url_value,
                          &line_value,
                          &column_value);
       break;
     }
     default:
       NOTREACHED();
       break;
   }
-  action_on_load_ = DevToolsToggleAction::Show();
 }
 
 void DevToolsWindow::UpdateTheme() {
   ThemeService* tp = ThemeServiceFactory::GetForProfile(profile_);
   DCHECK(tp);
 
   std::string command("InspectorFrontendAPI.setToolbarColors(\"" +
       SkColorToRGBAString(tp->GetColor(ThemeProperties::COLOR_TOOLBAR)) +
       "\", \"" +
       SkColorToRGBAString(tp->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT)) +
@@ skipping 64 matching lines @@
   web_contents_->GetRenderViewHost()->ExecuteJavascriptInWebFrame(
       base::string16(), javascript);
 }
 
 void DevToolsWindow::UpdateBrowserToolbar() {
   BrowserWindow* inspected_window = GetInspectedBrowserWindow();
   if (inspected_window)
     inspected_window->UpdateToolbar(NULL);
 }
 
-bool DevToolsWindow::IsDocked() {
-  return dock_side_ != DEVTOOLS_DOCK_SIDE_UNDOCKED;
-}
-
-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;
 }
 
 void DevToolsWindow::DocumentOnLoadCompletedInMainFrame() {
-  is_loaded_ = true;
+  load_state_mask_ |= kOnLoadFired;
+  if (load_state_mask_ == kLoadCompleted)
+    LoadCompleted();
+}
+
+void DevToolsWindow::LoadCompleted() {
+  Show(action_on_load_);
+  action_on_load_ = DevToolsToggleAction::Nothing();
   UpdateTheme();
-  DoAction();
   AddDevToolsExtensionsToClient();
+  if (set_is_docked_for_test_on_load_) {
+    set_is_docked_for_test_on_load_ = false;
+    base::FundamentalValue docked(is_docked_);
+    CallClientFunction("InspectorFrontendAPI.setIsDocked", &docked, NULL, NULL);
+  }
 }