Show the filebrowse ui rather than the download shelf in chromeos.

This cl displays the filebrowse ui rather than download shelf for
downloads in chrom(e|ium) os. It conditionally replaces (with
preprocessor macros) the Browser::OnStartDownload method to do this.

The cl adds a static FileBrowseUI::OpenPopup(profile, hashArgument),
which opens the file browse ui and passes it the provided hash argument.
This is invoked directly from Browser::OnStartDownload. The
USBMountObserver code was changed to call this static method, rather
than open the popup by hand as it had been doing.

I'm not sure about ownership of the Browser* returned by OpenPopup, but
based on other code in the tree I assume chrome will deal with freeing
it when appropriate.

Before this change, USBMountObserver would add the window to registrar_
*before* showing it. Now that FileBrowseUI::OpenPopup returns a which
which is already visible, this is no longer the case. I assume this
won't be a problem.

Commit this for rginda.
Original review: http://codereview.chromium.org/555167

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/564022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@38222 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 95 insertions, 26 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 9a02f04..e284f69 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -59,6 +59,7 @@
 #include "chrome/common/platform_util.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/test/automation/automation_messages.h"
 #include "chrome/test/automation/tab_proxy.h"
 #include "net/proxy/proxy_service.h"
@@ -1118,10 +1119,28 @@ void AutomationProvider::GetShelfVisibility(int handle, bool* visible) {
   *visible = false;
 
   if (browser_tracker_->ContainsHandle(handle)) {
+#if defined(OS_CHROMEOS)
+    // Chromium OS shows FileBrowse ui rather than download shelf. So we
+    // enumerate all browsers and look for a chrome://filebrowse... pop up.
+    for (BrowserList::const_iterator it = BrowserList::begin();
+         it != BrowserList::end(); ++it) {
+      if ((*it)->type() == Browser::TYPE_POPUP) {
+        const GURL& url =
+            (*it)->GetTabContentsAt((*it)->selected_index())->GetURL();
+
+        if (url.SchemeIs(chrome::kChromeUIScheme) &&
+            url.host() == chrome::kChromeUIFileBrowseHost) {
+          *visible = true;
+          break;
+        }
+      }
+    }
+#else
     Browser* browser = browser_tracker_->GetResource(handle);
     if (browser) {
       *visible = browser->window()->IsDownloadShelfVisible();
     }
+#endif
   }
 }
 
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 193b6ad..b22880b 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/character_encoding.h"
 #include "chrome/browser/debugger/devtools_manager.h"
 #include "chrome/browser/debugger/devtools_window.h"
+#include "chrome/browser/dom_ui/filebrowse_ui.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_shelf.h"
@@ -1061,7 +1062,14 @@ void Browser::ShowFindBar() {
 }
 
 bool Browser::SupportsWindowFeature(WindowFeature feature) const {
-  unsigned int features = FEATURE_INFOBAR | FEATURE_DOWNLOADSHELF;
+  unsigned int features = FEATURE_INFOBAR;
+
+#if !defined(OS_CHROMEOS)
+  // Chrome OS opens a FileBrowse pop up instead of using download shelf.
+  // So FEATURE_DOWNLOADSHELF is only added for non-chromeos platforms.
+  features |= FEATURE_DOWNLOADSHELF;
+#endif  // !defined(OS_CHROMEOS)
+
   if (type() == TYPE_NORMAL) {
     features |= FEATURE_BOOKMARKBAR;
     features |= FEATURE_EXTENSIONSHELF;
@@ -2252,6 +2260,12 @@ void Browser::OnStartDownload(DownloadItem* download) {
   if (!window())
     return;
 
+#if defined(OS_CHROMEOS)
+  // skip the download shelf and just open the file browser in chromeos
+  std::string arg = download->full_path().DirName().value();
+  FileBrowseUI::OpenPopup(profile_, arg);
+
+#else
   // GetDownloadShelf creates the download shelf if it was not yet created.
   window()->GetDownloadShelf()->AddDownload(new DownloadItemModel(download));
 
@@ -2267,8 +2281,10 @@ void Browser::OnStartDownload(DownloadItem* download) {
   TabContents* current_tab = GetSelectedTabContents();
   // We make this check for the case of minimized windows, unit tests, etc.
   if (platform_util::IsVisible(current_tab->GetNativeView()) &&
-      Animation::ShouldRenderRichAnimation())
+      Animation::ShouldRenderRichAnimation()) {
     DownloadStartedAnimation::Show(current_tab);
+  }
+#endif
 }
 
 void Browser::ConfirmAddSearchProvider(const TemplateURL* template_url,
diff --git a/chrome/browser/chromeos/usb_mount_observer.cc b/chrome/browser/chromeos/usb_mount_observer.cc
index d088334..7a271f4 100644
--- a/chrome/browser/chromeos/usb_mount_observer.cc
+++ b/chrome/browser/chromeos/usb_mount_observer.cc
@@ -6,12 +6,13 @@
 
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_window.h"
+#include "chrome/browser/dom_ui/filebrowse_ui.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 
 namespace chromeos {
 
 const char* kFilebrowseURLHash = "chrome://filebrowse#";
-const char* kFilebrowseURLScanning = "chrome://filebrowse#scanningdevice";
+const char* kFilebrowseScanning = "scanningdevice";
 const int kPopupLeft = 0;
 const int kPopupTop = 0;
 const int kPopupWidth = 250;
@@ -33,20 +34,12 @@ void USBMountObserver::Observe(NotificationType type,
   }
 }
 
-void USBMountObserver::PopUpWindow(const std::string& url,
-                                   const std::string& device_path) {
-  Browser* browser = Browser::CreateForPopup(profile_);
-  browser->AddTabWithURL(
-      GURL(url), GURL(), PageTransition::LINK,
-      true, -1, false, NULL);
-  browser->window()->SetBounds(gfx::Rect(kPopupLeft,
-                                         kPopupTop,
-                                         kPopupWidth,
-                                         kPopupHeight));
+void USBMountObserver::OpenFileBrowse(const std::string& url,
+                                      const std::string& device_path) {
+  Browser *browser = FileBrowseUI::OpenPopup(profile_, url);
   registrar_.Add(this,
                  NotificationType::BROWSER_CLOSED,
                  Source<Browser>(browser));
-  browser->window()->Show();
   BrowserWithPath new_browser;
   new_browser.browser = browser;
   new_browser.device_path = device_path;
@@ -85,9 +78,7 @@ void USBMountObserver::MountChanged(chromeos::MountLibrary* obj,
               iter->device_path = path;
               iter->browser->Reload();
             } else {
-              std::string url = kFilebrowseURLHash;
-              url += disks[i].mount_path;
-              PopUpWindow(url, disks[i].device_path);
+              OpenFileBrowse(disks[i].mount_path, disks[i].device_path);
             }
           }
           return;
@@ -97,8 +88,7 @@ void USBMountObserver::MountChanged(chromeos::MountLibrary* obj,
   } else if (evt == chromeos::DEVICE_ADDED) {
     LOG(INFO) << "Got device added" << path;
     // TODO(dhg): Refactor once mole api is ready.
-    std::string url = kFilebrowseURLScanning;
-    PopUpWindow(url, path);
+    OpenFileBrowse(kFilebrowseScanning, path);
   } else if (evt == chromeos::DEVICE_SCANNED) {
     LOG(INFO) << "Got device scanned:" << path;
   }
diff --git a/chrome/browser/chromeos/usb_mount_observer.h b/chrome/browser/chromeos/usb_mount_observer.h
index 7f11444..11e5f26 100644
--- a/chrome/browser/chromeos/usb_mount_observer.h
+++ b/chrome/browser/chromeos/usb_mount_observer.h
@@ -52,7 +52,7 @@ class USBMountObserver : public chromeos::MountLibrary::Observer,
 
   // Used to create a window of a standard size, and add it to a list
   // of tracked browser windows in case that device goes away.
-  void PopUpWindow(const std::string& url, const std::string& device_path);
+  void OpenFileBrowse(const std::string& url, const std::string& device_path);
 
   Profile* profile_;
   std::vector<BrowserWithPath> browsers_;
diff --git a/chrome/browser/dom_ui/filebrowse_ui.cc b/chrome/browser/dom_ui/filebrowse_ui.cc
index 103ab07..b6d4498 100644
--- a/chrome/browser/dom_ui/filebrowse_ui.cc
+++ b/chrome/browser/dom_ui/filebrowse_ui.cc
@@ -54,6 +54,12 @@ static const std::string kPicasawebDefault = "/albumid/default";
 static const std::string kPicasawebDropBox = "/DropBox";
 static const std::string kPicasawebBaseUrl = "http://picasaweb.google.com/";
 
+static const char* kFilebrowseURLHash = "chrome://filebrowse#";
+static const int kPopupLeft = 0;
+static const int kPopupTop = 0;
+static const int kPopupWidth = 250;
+static const int kPopupHeight = 300;
+
 class FileBrowseUIHTMLSource : public ChromeURLDataManager::DataSource {
  public:
   FileBrowseUIHTMLSource();
@@ -678,7 +684,7 @@ void FilebrowseHandler::SendCurrentDownloads() {
 
 ////////////////////////////////////////////////////////////////////////////////
 //
-// FileBrowseUIContents
+// FileBrowseUI
 //
 ////////////////////////////////////////////////////////////////////////////////
 
@@ -696,3 +702,24 @@ FileBrowseUI::FileBrowseUI(TabContents* contents) : HtmlDialogUI(contents) {
           &ChromeURLDataManager::AddDataSource,
           make_scoped_refptr(html_source)));
 }
+
+// static
+Browser *FileBrowseUI::OpenPopup(Profile *profile,
+                                 const std::string hashArgument) {
+  Browser* browser = Browser::CreateForPopup(profile);
+
+  std::string url(kFilebrowseURLHash);
+  url.append(hashArgument);
+
+  browser->AddTabWithURL(
+      GURL(url), GURL(), PageTransition::LINK,
+      true, -1, false, NULL);
+  browser->window()->SetBounds(gfx::Rect(kPopupLeft,
+                                         kPopupTop,
+                                         kPopupWidth,
+                                         kPopupHeight));
+
+  browser->window()->Show();
+
+  return browser;
+}
diff --git a/chrome/browser/dom_ui/filebrowse_ui.h b/chrome/browser/dom_ui/filebrowse_ui.h
index 5f45fe4..1c16536 100644
--- a/chrome/browser/dom_ui/filebrowse_ui.h
+++ b/chrome/browser/dom_ui/filebrowse_ui.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_DOM_UI_FILEBROWSE_UI_H_
 #define CHROME_BROWSER_DOM_UI_FILEBROWSE_UI_H_
 
+#include <string>
+
 #include "base/file_path.h"
 #include "base/scoped_ptr.h"
 #include "base/values.h"
@@ -14,12 +16,15 @@
 #include "net/base/directory_lister.h"
 
 
-class GURL;
+class Browser;
+class Profile;
 
 class FileBrowseUI : public HtmlDialogUI {
  public:
   explicit FileBrowseUI(TabContents* contents);
 
+  static Browser *OpenPopup(Profile *profile, const std::string hashArgument);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(FileBrowseUI);
 };
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index dac35d3..142ed9a 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -88,7 +88,10 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnly) {
   SavePageFinishedObserver observer;
 
   EXPECT_EQ(url, observer.page_url());
-  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::ContentsEqual(
@@ -113,7 +116,10 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
   SavePageFinishedObserver observer;
 
   EXPECT_EQ(url, observer.page_url());
-  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_TRUE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::TextContentsEqual(
@@ -153,7 +159,10 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, FileNameFromPageTitle) {
   SavePageFinishedObserver observer;
 
   EXPECT_EQ(url, observer.page_url());
-  EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
+  if (browser()->SupportsWindowFeature(Browser::FEATURE_DOWNLOADSHELF))
+    EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
+
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_TRUE(file_util::PathExists(dir));
   EXPECT_TRUE(file_util::TextContentsEqual(
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 246e8f0..7ee002d 100755
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -71,7 +71,7 @@
         #'browser/net/url_request_mock_http_job.cc',
         #'browser/net/url_request_mock_http_job.h',
         'browser/net/url_request_mock_net_error_job.cc',
-        'browser/net/url_request_mock_net_error_job.h',		
+        'browser/net/url_request_mock_net_error_job.h',
         'browser/renderer_host/mock_render_process_host.cc',
         'browser/renderer_host/mock_render_process_host.h',
         'browser/renderer_host/test/test_backing_store.cc',
@@ -341,6 +341,9 @@
           'dependencies': [
             '../views/views.gyp:views',
           ],
+          'sources!': [
+            'browser/download/download_uitest.cc',
+          ],
         }],
         ['OS=="mac"', {
           'sources!': [