Add switch to provide app window size on startup

chrome/browser/ui/startup/startup_browser_creator_impl.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/ui/startup/startup_browser_creator_impl.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ skipping 67 matching lines @@
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "grit/locale_settings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/screen.h"
+
+#if defined(USE_ASH)
+#include "ash/launcher/launcher_types.h"
+#endif
 
 #if defined(OS_MACOSX)
 #include "base/mac/mac_util.h"
 #include "chrome/browser/ui/cocoa/keystone_infobar_delegate.h"
 #endif
 
 #if defined(TOOLKIT_GTK)
 #include "chrome/browser/ui/gtk/gtk_util.h"
 #endif
 
@@ skipping 104 matching lines @@
   // preference is set, launch as a window.
   extension_misc::LaunchContainer launch_container =
       extensions_service->extension_prefs()->GetLaunchContainer(
           extension, extensions::ExtensionPrefs::LAUNCH_WINDOW);
 
   *out_extension = extension;
   *out_launch_container = launch_container;
   return true;
 }
 
+// Parse two comma-separated integers from str. Return true on success.
+bool ParseCommaSeparatedIntegers(const std::string& str,
+                                 int* ret_num1,
+                                 int* ret_num2) {
+  size_t num1_size = str.find_first_of(',');

[sky, 2012/09/19 17:20:14]

Seems like this would be a lot simpler if you used SplitString.

[Nikita (slow), 2012/09/20 15:01:27]

Done.

+  if (num1_size == std::string::npos)
+    return false;
+
+  size_t num2_pos = num1_size + 1;
+  size_t num2_size = str.size() - num2_pos;
+  int num1, num2;
+  if (!base::StringToInt(str.substr(0, num1_size), &num1) ||
+      !base::StringToInt(str.substr(num2_pos, num2_size), &num2))
+    return false;
+
+  *ret_num1 = num1;
+  *ret_num2 = num2;
+  return true;
+}
+
 void RecordCmdLineAppHistogram() {
   AppLauncherHandler::RecordAppLaunchType(
       extension_misc::APP_LAUNCH_CMD_LINE_APP);
 }
 
 void RecordAppLaunches(Profile* profile,
                        const std::vector<GURL>& cmd_line_urls,
                        StartupTabs& autolaunch_tabs) {
   ExtensionService* extension_service = profile->GetExtensionService();
   DCHECK(extension_service);
@@ skipping 280 matching lines @@
         ChildProcessSecurityPolicy::GetInstance();
     if (policy->IsWebSafeScheme(url.scheme()) ||
         url.SchemeIs(chrome::kFileScheme)) {
       if (profile->GetExtensionService()->IsInstalledApp(url)) {
         RecordCmdLineAppHistogram();
       } else {
         AppLauncherHandler::RecordAppLaunchType(
             extension_misc::APP_LAUNCH_CMD_LINE_APP_LEGACY);
       }
 
+      gfx::Rect override_bounds;
+
+      if (command_line_.HasSwitch(switches::kAppWindowSize)) {

[sky, 2012/09/19 17:20:14]

Move this into its own function.

[Nikita (slow), 2012/09/20 15:01:27]

Done.

+        int width, height;
+        std::string switch_value =
+            command_line_.GetSwitchValueASCII(switches::kAppWindowSize);
+        if (ParseCommaSeparatedIntegers(switch_value, &width, &height)) {
+          override_bounds.set_size(gfx::Size(width, height));

[sky, 2012/09/19 17:20:14]

Make sure the size fits in the work area too.

[Nikita (slow), 2012/09/20 15:01:27]

Not sure if this is really needs to be done here as window will be sized to
working area size even if I pass something like 3000x3000

[Nikita (slow), 2012/09/20 15:01:27]

Done.

+          const gfx::Rect work_area =
+              gfx::Screen::GetPrimaryDisplay().work_area();
+          override_bounds.set_x(
+              (work_area.width() - override_bounds.width()) / 2);
+          int extra_height_work_area = 0;
+#if defined(USE_ASH)
+          // In case of Ash we should center app window on the rest of the
+          // working area outside of launcher.
+          extra_height_work_area = ash::kLauncherPreferredSize;

[sky, 2012/09/19 17:20:14]

The work area shouldn't include kLauncherPreferredSize, how come you need to do
this?

[Nikita (slow), 2012/09/19 17:46:42]

Probably you're right but from what I've seen on linux dev machine it does
include launcher. That could be one of 2:

1. We're grabbing work_area() before launcher has been added into layout
2. chromeos Chrome default window size height is larger then  laptop formfactor
so it does include launcher into work area.

I'll check on device (lumpy).

[Nikita (slow), 2012/09/20 16:55:00]

Removed for now.
Work area should not include launcher but it does now.

+#endif
+          override_bounds.set_y((work_area.height() -
+              override_bounds.height() - extra_height_work_area) / 2);
+        }
+      }
+
       WebContents* app_tab = application_launch::OpenAppShortcutWindow(
           profile,
-          url);
+          url,
+          override_bounds);
 
       if (out_app_contents)
         *out_app_contents = app_tab;
 
       return (app_tab != NULL);
     }
   }
   return false;
 }
 
@@ skipping 417 matching lines @@
         // New:
         prefs->GetString(prefs::kHomePage),
         prefs->GetBoolean(prefs::kHomePageIsNewTabPage),
         prefs->GetBoolean(prefs::kShowHomeButton),
         // Backup:
         backup_homepage,
         backup_homepage_is_ntp,
         backup_show_home_button));
   }
 }