Revert 157174 - Limit number of code paths for accessing screenshot sources to avoid potential future pitfalls.

[Compile failure. Some header moved, apparently.]

BUG=130932


Review URL: https://chromiumcodereview.appspot.com/10908081

TBR=harrym@chromium.org
Review URL: https://codereview.chromium.org/10914324

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

4 files changed, 105 insertions, 162 deletions

diff --git a/chrome/browser/ui/ash/screenshot_taker.cc b/chrome/browser/ui/ash/screenshot_taker.cc
index 9bb5b1d..f83dcfd 100644
--- a/chrome/browser/ui/ash/screenshot_taker.cc
+++ b/chrome/browser/ui/ash/screenshot_taker.cc
@@ -13,22 +13,24 @@
 #include "base/bind.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
+#include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/stringprintf.h"
 #include "base/time.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/ui/webui/screenshot_source.h"
 #include "chrome/browser/ui/window_snapshot/window_snapshot.h"
+#include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/gdata/drive_file_system_util.h"
-#include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #endif
 
@@ -44,6 +46,74 @@ const int64 kVisualFeedbackLayerDisplayTimeMs = 100;
 // more than 1000 to prevent the conflict of filenames.
 const int kScreenshotMinimumIntervalInMS = 1000;
 
+bool ShouldUse24HourClock() {
+#if defined(OS_CHROMEOS)
+  Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
+  if (profile) {
+    PrefService* pref_service = profile->GetPrefs();
+    if (pref_service) {
+      return pref_service->GetBoolean(prefs::kUse24HourClock);
+    }
+  }
+#endif
+  return base::GetHourClockType() == base::k24HourClock;
+}
+
+bool AreScreenshotsDisabled() {
+  return g_browser_process->local_state()->GetBoolean(
+      prefs::kDisableScreenshots);
+}
+
+std::string GetScreenshotBaseFilename() {
+  base::Time::Exploded now;
+  base::Time::Now().LocalExplode(&now);
+
+  // We don't use base/i18n/time_formatting.h here because it doesn't
+  // support our format.  Don't use ICU either to avoid i18n file names
+  // for non-English locales.
+  // TODO(mukai): integrate this logic somewhere time_formatting.h
+  std::string file_name = base::StringPrintf(
+      "Screenshot %d-%02d-%02d at ", now.year, now.month, now.day_of_month);
+
+  if (ShouldUse24HourClock()) {
+    file_name.append(base::StringPrintf(
+        "%02d.%02d.%02d", now.hour, now.minute, now.second));
+  } else {
+    int hour = now.hour;
+    if (hour > 12) {
+      hour -= 12;
+    } else if (hour == 0) {
+      hour = 12;
+    }
+    file_name.append(base::StringPrintf(
+        "%d.%02d.%02d ", hour, now.minute, now.second));
+    file_name.append((now.hour >= 12) ? "PM" : "AM");
+  }
+
+  return file_name;
+}
+
+bool GetScreenshotDirectory(FilePath* directory) {
+  if (AreScreenshotsDisabled())
+    return false;
+
+  bool is_logged_in = true;
+#if defined(OS_CHROMEOS)
+  is_logged_in = chromeos::UserManager::Get()->IsUserLoggedIn();
+#endif
+
+  if (is_logged_in) {
+    DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext(
+        ash::Shell::GetInstance()->delegate()->GetCurrentBrowserContext());
+    *directory = download_prefs->DownloadPath();
+  } else {
+    if (!file_util::GetTempDir(directory)) {
+      LOG(ERROR) << "Failed to find temporary directory.";
+      return false;
+    }
+  }
+  return true;
+}
 
 void SaveScreenshot(const FilePath& screenshot_path,
                     scoped_refptr<base::RefCountedBytes> png_data) {
@@ -100,7 +170,7 @@ bool GrabWindowSnapshot(aura::Window* window,
 #if defined(OS_LINUX)
   // chrome::GrabWindowSnapshotForUser checks this too, but
   // RootWindow::GrabSnapshot does not.
-  if (ScreenshotSource::AreScreenshotsDisabled())
+  if (AreScreenshotsDisabled())
     return false;
 
   // We use XGetImage() for Linux/ChromeOS for performance reasons.
@@ -123,11 +193,10 @@ ScreenshotTaker::~ScreenshotTaker() {
 
 void ScreenshotTaker::HandleTakeScreenshotForAllRootWindows() {
   FilePath screenshot_directory;
-  if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory))
+  if (!GetScreenshotDirectory(&screenshot_directory))
     return;
 
-  std::string screenshot_basename =
-      ScreenshotSource::GetScreenshotBaseFilename();
+  std::string screenshot_basename = GetScreenshotBaseFilename();
   ash::Shell::RootWindowList root_windows = ash::Shell::GetAllRootWindows();
   for (size_t i = 0; i < root_windows.size(); ++i) {
     aura::RootWindow* root_window = root_windows[i];
@@ -152,7 +221,7 @@ void ScreenshotTaker::HandleTakePartialScreenshot(
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
   FilePath screenshot_directory;
-  if (!ScreenshotSource::GetScreenshotDirectory(&screenshot_directory))
+  if (!GetScreenshotDirectory(&screenshot_directory))
     return;
 
   scoped_refptr<base::RefCountedBytes> png_data(new base::RefCountedBytes);
@@ -161,9 +230,8 @@ void ScreenshotTaker::HandleTakePartialScreenshot(
     last_screenshot_timestamp_ = base::Time::Now();
     DisplayVisualFeedback(rect);
     PostSaveScreenshotTask(
-        screenshot_directory.AppendASCII(
-                   ScreenshotSource::GetScreenshotBaseFilename() + ".png"),
-                   png_data);
+        screenshot_directory.AppendASCII(GetScreenshotBaseFilename() + ".png"),
+        png_data);
   } else {
     LOG(ERROR) << "Failed to grab the window screenshot";
   }
diff --git a/chrome/browser/ui/webui/feedback_ui.cc b/chrome/browser/ui/webui/feedback_ui.cc
index 6636b4d..ec0846b 100644
--- a/chrome/browser/ui/webui/feedback_ui.cc
+++ b/chrome/browser/ui/webui/feedback_ui.cc
@@ -76,6 +76,9 @@ using ui::WebDialogUI;
 
 namespace {
 
+const char kScreenshotBaseUrl[] = "chrome://screenshots/";
+const char kCurrentScreenshotUrl[] = "chrome://screenshots/current";
+
 const char kCategoryTagParameter[] = "categoryTag=";
 const char kDescriptionParameter[] = "description=";
 const char kSessionIDParameter[] = "session_id=";
@@ -83,10 +86,16 @@ const char kTabIndexParameter[] = "tab_index=";
 const char kCustomPageUrlParameter[] = "customPageUrl=";
 
 #if defined(OS_CHROMEOS)
+const char kSavedScreenshotsUrl[] = "chrome://screenshots/saved/";
+const char kScreenshotPrefix[] = "Screenshot ";
+const char kScreenshotSuffix[] = ".png";
 
 const char kTimestampParameter[] = "timestamp=";
 
 const size_t kMaxSavedScreenshots = 2;
+#endif
+
+#if defined(OS_CHROMEOS)
 size_t kMaxNumScanFiles = 1000;
 
 // Compare two screenshot filepaths, which include the screenshot timestamp
@@ -126,10 +135,8 @@ void ReadDirectoryCallback(size_t max_saved,
   std::vector<gdata::DriveEntryProto> screenshot_entries;
   for (size_t i = 0; i < max_scan; ++i) {
     const gdata::DriveEntryProto& entry = (*entries)[i];
-    if (StartsWithASCII(entry.base_name(),
-                        ScreenshotSource::kScreenshotPrefix, true) &&
-        EndsWith(entry.base_name(),
-                 ScreenshotSource::kScreenshotSuffix, true)) {
+    if (StartsWithASCII(entry.base_name(), kScreenshotPrefix, true) &&
+        EndsWith(entry.base_name(), kScreenshotSuffix, true)) {
       screenshot_entries.push_back(entry);
     }
   }
@@ -142,9 +149,7 @@ void ReadDirectoryCallback(size_t max_saved,
   for (size_t i = 0; i < sort_size; ++i) {
     const gdata::DriveEntryProto& entry = screenshot_entries[i];
     saved_screenshots->push_back(
-        std::string(ScreenshotSource::kScreenshotUrlRoot) +
-        std::string(ScreenshotSource::kScreenshotSaved) +
-        entry.resource_id());
+        std::string(kSavedScreenshotsUrl) + entry.resource_id());
   }
   callback.Run();
 }
@@ -543,9 +548,7 @@ void FeedbackHandler::HandleGetDialogDefaults(const ListValue*) {
 }
 
 void FeedbackHandler::HandleRefreshCurrentScreenshot(const ListValue*) {
-  std::string current_screenshot(
-          std::string(ScreenshotSource::kScreenshotUrlRoot) +
-          std::string(ScreenshotSource::kScreenshotCurrent));
+  std::string current_screenshot(kCurrentScreenshotUrl);
   StringValue screenshot(current_screenshot);
   web_ui()->CallJavascriptFunction("setupCurrentScreenshot", screenshot);
 }
@@ -618,7 +621,7 @@ void FeedbackHandler::HandleSendReport(const ListValue* list_value) {
   (*i++)->GetAsString(&user_email);
   std::string screenshot_path;
   (*i++)->GetAsString(&screenshot_path);
-  screenshot_path.erase(0, strlen(ScreenshotSource::kScreenshotUrlRoot));
+  screenshot_path.erase(0, strlen(kScreenshotBaseUrl));
 
   // Get the image to send in the report.
   ScreenshotDataPtr image_ptr;
@@ -729,8 +732,7 @@ void FeedbackUI::GetMostRecentScreenshots(
     std::vector<std::string>* saved_screenshots,
     size_t max_saved) {
   std::string pattern =
-      std::string(ScreenshotSource::kScreenshotPrefix) + "*" +
-                  ScreenshotSource::kScreenshotSuffix;
+      std::string(kScreenshotPrefix) + "*" + kScreenshotSuffix;
   file_util::FileEnumerator screenshots(filepath, false,
                                         file_util::FileEnumerator::FILES,
                                         pattern);
@@ -748,9 +750,7 @@ void FeedbackUI::GetMostRecentScreenshots(
                     screenshot_filepaths.end(),
                     ScreenshotTimestampComp);
   for (size_t i = 0; i < sort_size; ++i)
-    saved_screenshots->push_back(
-                    std::string(ScreenshotSource::kScreenshotUrlRoot) +
-                    std::string(ScreenshotSource::kScreenshotSaved) +
-                    screenshot_filepaths[i]);
+    saved_screenshots->push_back(std::string(kSavedScreenshotsUrl) +
+                                   screenshot_filepaths[i]);
 }
 #endif
diff --git a/chrome/browser/ui/webui/screenshot_source.cc b/chrome/browser/ui/webui/screenshot_source.cc
index 311adc2..f4ba7ada 100644
--- a/chrome/browser/ui/webui/screenshot_source.cc
+++ b/chrome/browser/ui/webui/screenshot_source.cc
@@ -6,63 +6,32 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/file_path.h"
 #include "base/file_util.h"
-#include "base/i18n/time_formatting.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string16.h"
-#include "base/stringprintf.h"
 #include "base/string_util.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_prefs.h"
-#include "chrome/browser/prefs/pref_service.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_paths.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/url_canon.h"
 #include "googleurl/src/url_util.h"
 
-#if defined(USE_ASH)
+#if defined(OS_CHROMEOS)
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
-#endif
-
-#if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/gdata/drive_file_system_interface.h"
 #include "chrome/browser/chromeos/gdata/drive_file_system_util.h"
 #include "chrome/browser/chromeos/gdata/drive_system_service.h"
-#include "chrome/browser/chromeos/gdata/gdata_util.h"
-#include "chrome/browser/chromeos/login/user_manager.h"
 #include "content/public/browser/browser_thread.h"
 #endif
 
-// static
-const char ScreenshotSource::kScreenshotUrlRoot[] = "chrome://screenshots/";
-// static
-const char ScreenshotSource::kScreenshotCurrent[] = "current";
-// static
-const char ScreenshotSource::kScreenshotSaved[] = "saved/";
+static const char kCurrentScreenshotFilename[] = "current";
 #if defined(OS_CHROMEOS)
-// static
-const char ScreenshotSource::kScreenshotPrefix[] = "Screenshot ";
-// static
-const char ScreenshotSource::kScreenshotSuffix[] = ".png";
+static const char kSavedScreenshotsBasePath[] = "saved/";
 #endif
 
-bool ShouldUse24HourClock() {
-#if defined(OS_CHROMEOS)
-  Profile* profile = ProfileManager::GetDefaultProfileOrOffTheRecord();
-  if (profile) {
-    return profile->GetPrefs()->GetBoolean(prefs::kUse24HourClock);
-  }
-#endif
-  return base::GetHourClockType() == base::k24HourClock;
-}
-
 ScreenshotSource::ScreenshotSource(
     std::vector<unsigned char>* current_screenshot,
     Profile* profile)
@@ -77,70 +46,6 @@ ScreenshotSource::ScreenshotSource(
 
 ScreenshotSource::~ScreenshotSource() {}
 
-// static
-std::string ScreenshotSource::GetScreenshotBaseFilename() {
-  base::Time::Exploded now;
-  base::Time::Now().LocalExplode(&now);
-
-  // We don't use base/i18n/time_formatting.h here because it doesn't
-  // support our format.  Don't use ICU either to avoid i18n file names
-  // for non-English locales.
-  // TODO(mukai): integrate this logic somewhere time_formatting.h
-  std::string file_name = base::StringPrintf(
-      "Screenshot %d-%02d-%02d at ", now.year, now.month, now.day_of_month);
-
-  if (ShouldUse24HourClock()) {
-    file_name.append(base::StringPrintf(
-        "%02d.%02d.%02d", now.hour, now.minute, now.second));
-  } else {
-    int hour = now.hour;
-    if (hour > 12) {
-      hour -= 12;
-    } else if (hour == 0) {
-      hour = 12;
-    }
-    file_name.append(base::StringPrintf(
-        "%d.%02d.%02d ", hour, now.minute, now.second));
-    file_name.append((now.hour >= 12) ? "PM" : "AM");
-  }
-
-  return file_name;
-}
-
-#if defined(USE_ASH)
-
-// static
-bool ScreenshotSource::AreScreenshotsDisabled() {
-  return g_browser_process->local_state()->GetBoolean(
-      prefs::kDisableScreenshots);
-}
-
-// static
-bool ScreenshotSource::GetScreenshotDirectory(FilePath* directory) {
-  if (ScreenshotSource::AreScreenshotsDisabled())
-    return false;
-
-  bool is_logged_in = true;
-
-#if defined(OS_CHROMEOS)
-  is_logged_in = chromeos::UserManager::Get()->IsUserLoggedIn();
-#endif
-
-  if (is_logged_in) {
-    DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext(
-        ash::Shell::GetInstance()->delegate()->GetCurrentBrowserContext());
-    *directory = download_prefs->DownloadPath();
-  } else  {
-    if (!file_util::GetTempDir(directory)) {
-      LOG(ERROR) << "Failed to find temporary directory.";
-      return false;
-    }
-  }
-  return true;
-}
-
-#endif
-
 void ScreenshotSource::StartDataRequest(const std::string& path, bool,
                                         int request_id) {
   SendScreenshot(path, request_id);
@@ -170,15 +75,14 @@ void ScreenshotSource::SendScreenshot(const std::string& screenshot_path,
   // image gets reloaded instead of being pulled from the browser cache
   std::string path = screenshot_path.substr(
       0, screenshot_path.find_first_of("?"));
-  if (path == ScreenshotSource::kScreenshotCurrent) {
+  if (path == kCurrentScreenshotFilename) {
     CacheAndSendScreenshot(path, request_id, current_screenshot_);
 #if defined(OS_CHROMEOS)
-  } else if (path.compare(0, strlen(ScreenshotSource::kScreenshotSaved),
-             ScreenshotSource::kScreenshotSaved) == 0) {
+  } else if (path.compare(0, strlen(kSavedScreenshotsBasePath),
+                          kSavedScreenshotsBasePath) == 0) {
     using content::BrowserThread;
 
-    std::string filename =
-                       path.substr(strlen(ScreenshotSource::kScreenshotSaved));
+    std::string filename = path.substr(strlen(kSavedScreenshotsBasePath));
 
     url_canon::RawCanonOutputT<char16> decoded;
     url_util::DecodeURLEscapeSequences(
@@ -187,8 +91,9 @@ void ScreenshotSource::SendScreenshot(const std::string& screenshot_path,
     std::string decoded_filename = UTF16ToASCII(string16(
         decoded.data(), decoded.length()));
 
-    FilePath download_path;
-    GetScreenshotDirectory(&download_path);
+    DownloadPrefs* download_prefs = DownloadPrefs::FromBrowserContext(
+        ash::Shell::GetInstance()->delegate()->GetCurrentBrowserContext());
+    FilePath download_path = download_prefs->DownloadPath();
     if (gdata::util::IsUnderDriveMountPoint(download_path)) {
       gdata::DriveFileSystemInterface* file_system =
           gdata::DriveSystemServiceFactory::GetForProfile(
diff --git a/chrome/browser/ui/webui/screenshot_source.h b/chrome/browser/ui/webui/screenshot_source.h
index bc4e902..b294a2d 100644
--- a/chrome/browser/ui/webui/screenshot_source.h
+++ b/chrome/browser/ui/webui/screenshot_source.h
@@ -15,8 +15,6 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/gdata/drive_resource_metadata.h"
-#include "chrome/browser/chromeos/gdata/gdata_util.h"
-#include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/google_apis/gdata_errorcode.h"
 #endif
 
@@ -34,19 +32,6 @@ class ScreenshotSource : public ChromeURLDataManager::DataSource {
       std::vector<unsigned char>* current_screenshot,
       Profile* profile);
 
-#if defined(USE_ASH)
-
-  // Queries the browser process to determine if screenshots are disabled.
-  static bool AreScreenshotsDisabled();
-
-  // Common access for the screenshot directory, parameter is set to the
-  // requested directory and return value of true is given upon success.
-  static bool GetScreenshotDirectory(FilePath* directory);
-#endif
-
-  // Get the basefilename for screenshots
-  static std::string GetScreenshotBaseFilename();
-
   // Called when the network layer has requested a resource underneath
   // the path we registered.
   virtual void StartDataRequest(const std::string& path,
@@ -60,21 +45,6 @@ class ScreenshotSource : public ChromeURLDataManager::DataSource {
   // Note: This method strips the query string from the given path.
   ScreenshotDataPtr GetCachedScreenshot(const std::string& screenshot_path);
 
-  // Url that represents the base directory for screenshots.
-  static const char kScreenshotUrlRoot[];
-  // Identifier for the current screenshot
-  // (relative to screenshot base directory).
-  static const char kScreenshotCurrent[];
-  // Path for directory where screenshots are saved
-  // (relative to screenshot base directory).
-  static const char kScreenshotSaved[];
-#if defined(OS_CHROMEOS)
-  // Common prefix to screenshot filenames.
-  static const char kScreenshotPrefix[];
-  // Common suffix to screenshot filenames.
-  static const char kScreenshotSuffix[];
-#endif
-
  private:
   virtual ~ScreenshotSource();