Fixing problem with browser invocations from a visiting files application

chrome/browser/chromeos/file_manager/open_with_browser.cc

Index: chrome/browser/chromeos/file_manager/open_with_browser.cc
diff --git a/chrome/browser/chromeos/file_manager/open_with_browser.cc b/chrome/browser/chromeos/file_manager/open_with_browser.cc
index 8b4e44c8e7f56934e0ee7a293f4cc5e212e15ceb..9d801837dc1a9a699a94dc3e5f65f75e8a2d4041 100644
--- a/chrome/browser/chromeos/file_manager/open_with_browser.cc
+++ b/chrome/browser/chromeos/file_manager/open_with_browser.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/chromeos/drive/file_system_util.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -107,6 +108,11 @@ void OpenNewTab(Profile* profile, const GURL& url) {
       profile, chrome::HOST_DESKTOP_TYPE_ASH);
   chrome::AddSelectedTabWithURL(displayer.browser(), url,
       content::PAGE_TRANSITION_LINK);
+
+  // Since the ScopedTabbedBrowserDisplayer does not guarantee that the

[mtomasz, 2014/01/30 01:47:18]

If there is already a window with lots of tabs for user A on desktop A, and we
are using a teleported Files App to desktop B, then the tab will be added to the
last active window for user A, and it will teleport it to desktop B together
with all of the tabs in this window. Am I correct?

If so, then shouldn't we look for a browser window of profile B on desktop B
first? If it exists, then create a tab there. If not, create a new browser
window for profile A on desktop B and add a tab there to avoid teleporting all
of the tabs which were on desktop A?

The code would be more complicated, but I guess matching better user
expectations. WDYT?

[Mr4D (OOO till 08-26), 2014/01/30 16:49:58]

I have spent yesterday already about 2 hours of implementing a new
ScopedTabbedBrowserDisplayer for this module to do exactly that. It enumerated
all windows from that user and picked a window which would be also on that
desktop - or create a new one.

However... I realized then that once a window is on the other desktop, that will
be chosen from then on as the LRU window. Somehow the effort to get that in
outweighs the benefit of doing it so I threw that code away. Furthermore we
informed the user that some odd things might happen when he teleports windows
and this solution seems to be much more acceptable then the dialog we discussed
yesterday.

If you disagree - let me know and I re-implement the displayer again. :)

[mtomasz, 2014/01/31 01:12:42]

SGTM. We can improve the logic in the future, if users don't like it.

+  // browser will be show on the active desktop, we ensure the visibility.

[mtomasz, 2014/01/31 01:12:42]

nit: show -> shown

[Mr4D (OOO till 08-26), 2014/01/31 15:20:58]

Done.

+  multi_user_util::MoveWindowToCurrentDesktop(
+      displayer.browser()->window()->GetNativeWindow());
 }
 
 // Reads the alternate URL from a GDoc file. When it fails, returns a file URL