Zoom Extension API

chrome/browser/extensions/api/tabs/tabs_api.cc

Index: chrome/browser/extensions/api/tabs/tabs_api.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index 39607d2b0f7f2532c3d2d37871f069edea697e07..767a79e014b0a229b3ee6e3d4ea8416a82a29b5f 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -44,6 +44,7 @@
 #include "chrome/browser/ui/panels/panel_manager.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/window_sizer/window_sizer.h"
+#include "chrome/browser/ui/zoom/zoom_controller.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/api/i18n/default_locale_handler.h"
@@ -60,6 +61,7 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"

[not at google - send to devlin, 2014/04/11 15:04:25]

no longer needed

[paulmeyer, 2014/04/11 19:09:49]

Done.

 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -215,6 +217,29 @@ Browser* CreateBrowserWindow(const Browser::CreateParams& params,
 
 }  // namespace
 
+void ZoomModeToZoomSettings(content::ZoomMode zoom_mode,
+                            api::tabs::ZoomSettings* zoom_settings) {
+  DCHECK(zoom_settings);
+  switch (zoom_mode) {
+    case content::kZoomModeDefault:
+      zoom_settings->mode = api::tabs::ZoomSettings::MODE_AUTOMATIC;
+      zoom_settings->scope = api::tabs::ZoomSettings::SCOPE_PER_ORIGIN;
+      break;

[not at google - send to devlin, 2014/04/11 15:04:25]

let's just return an api::tabs::ZoomSettings (or a scoped_ptr<> if it's
disallow-copy; but it's such a tiny struct that copying it is not a big deal)
rather than having it as an out-param.

[paulmeyer, 2014/04/11 19:09:49]

Yeah, it is disallow-copy, and being scoped_ptr isn't enough to fix that because
it still needs to be copied in tabs_event_router before being passed into
Create, so I think it's still better to leave it as an out-param.

On 2014/04/11 15:04:25, kalman wrote:

+    case content::kZoomModeIsolated:
+      zoom_settings->mode = api::tabs::ZoomSettings::MODE_AUTOMATIC;
+      zoom_settings->scope = api::tabs::ZoomSettings::SCOPE_PER_TAB;
+      break;
+    case content::kZoomModeManual:
+      zoom_settings->mode = api::tabs::ZoomSettings::MODE_MANUAL;
+      zoom_settings->scope = api::tabs::ZoomSettings::SCOPE_PER_TAB;
+      break;
+    case content::kZoomModeDisabled:
+      zoom_settings->mode = api::tabs::ZoomSettings::MODE_DISABLED;
+      zoom_settings->scope = api::tabs::ZoomSettings::SCOPE_PER_TAB;
+      break;
+  }
+}
+
 // Windows ---------------------------------------------------------------------
 
 bool WindowsGetFunction::RunImpl() {
@@ -1596,8 +1621,9 @@ bool TabsReloadFunction::RunImpl() {
                     NULL,
                     &web_contents,
                     NULL,
-                    &error_))
-    return false;
+                    &error_)) {
+      return false;
+    }
   }
 
   if (web_contents->ShowingInterstitialPage()) {
@@ -1917,4 +1943,150 @@ bool TabsInsertCSSFunction::ShouldInsertCSS() const {
   return true;
 }
 
+bool ZoomAPIFunction::GetWebContents(int* tab_id,
+                                     content::WebContents** web_contents) {
+  if (tab_id) {
+    if (!GetTabById(*tab_id,
+                    GetProfile(),
+                    include_incognito(),
+                    NULL,
+                    NULL,
+                    web_contents,
+                    NULL,
+                    &error_)) {
+      return false;
+    }
+  } else {
+    Browser* browser = GetCurrentBrowser();
+    if (!browser) {
+      error_ = keys::kNoCurrentWindowError;
+      return false;
+    }
+    if (!ExtensionTabUtil::GetDefaultTab(browser, web_contents, NULL)) {
+      error_ = keys::kNoCurrentTabError;

[not at google - send to devlin, 2014/04/11 15:04:25]

as discussed, you could re-use the "no selected tab" error.

[paulmeyer, 2014/04/11 19:09:49]

Done.

+      return false;
+    }
+  }
+  return true;
+}
+
+bool TabsSetZoomFunction::RunImpl() {
+  scoped_ptr<tabs::SetZoom::Params> params(
+      tabs::SetZoom::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  WebContents* web_contents = NULL;
+  if (!GetWebContents(params->tab_id.get(), &web_contents)) {
+    return false;
+  } else if (web_contents->GetURL().SchemeIs(content::kChromeUIScheme)) {

[not at google - send to devlin, 2014/04/11 15:04:25]

no else after return

[paulmeyer, 2014/04/11 19:09:49]

Done.

+    error_ = keys::kCannotZoomChromePagesError;

[not at google - send to devlin, 2014/04/11 15:04:25]

you should be able to reuse some sort of generic "not allowed' message here.
ideally not give away information like this being a chrome:// url.

[paulmeyer, 2014/04/11 19:09:49]

There aren't any existing errors that general, though if you'd like I could
simply remove the mention of chrome:// from mine.

On 2014/04/11 15:04:25, kalman wrote:

[not at google - send to devlin, 2014/04/11 20:37:02]

Ok, looks like we do have a precedent here in manifest_constants.h; let's just
use that:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/common/...

+    return false;
+  }
+
+  ZoomController* zoom_controller =
+      ZoomController::FromWebContents(web_contents);
+  double zoom_level = content::ZoomFactorToZoomLevel(params->zoom_factor);
+
+  // Attempt to set the zoom level with a callback.

[not at google - send to devlin, 2014/04/11 15:04:25]

comment doesn't add much

[paulmeyer, 2014/04/11 19:09:49]

Done.

+  scoped_refptr<TabsSetZoomFunction> this_ref = this;
+  if (!zoom_controller->SetZoomLevelByExtension(
+          zoom_level,
+          GetExtension(),
+          base::Bind(&TabsSetZoomFunction::SendResponse, this_ref, true))) {
+    // Tried to zoom a tab in disabled mode.
+    error_ = keys::kCannotZoomDisabledTabError;
+    return false;
+  }
+
+  return true;
+}
+
+bool TabsGetZoomFunction::RunImpl() {
+  scoped_ptr<tabs::GetZoom::Params> params(
+      tabs::GetZoom::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  WebContents* web_contents = NULL;
+  if (!GetWebContents(params->tab_id.get(), &web_contents)) {
+    return false;
+  }
+
+  double zoom_level = web_contents->GetZoomLevel();
+  double zoom_factor = content::ZoomLevelToZoomFactor(zoom_level);
+  results_ = tabs::GetZoom::Results::Create(zoom_factor);
+  SendResponse(true);
+  return true;
+}
+
+bool TabsSetZoomSettingsFunction::RunImpl() {
+  scoped_ptr<tabs::SetZoomSettings::Params> params(
+      tabs::SetZoomSettings::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  WebContents* web_contents = NULL;
+  if (!GetWebContents(params->tab_id.get(), &web_contents)) {
+    return false;
+  } else if (web_contents->GetURL().SchemeIs(content::kChromeUIScheme)) {

[not at google - send to devlin, 2014/04/11 15:04:25]

no else after return

[paulmeyer, 2014/04/11 19:09:49]

Done.

+    error_ = keys::kCannotChangeChromePageZoomSettingsError;

[not at google - send to devlin, 2014/04/11 15:04:25]

same comment re chrome:// url

[paulmeyer, 2014/04/11 19:09:49]

See above.

On 2014/04/11 15:04:25, kalman wrote:

+    return false;
+  }
+
+  // "per-origin" scope is only available in "automatic" mode.
+  if (params->zoom_settings.scope ==
+      api::tabs::ZoomSettings::SCOPE_PER_ORIGIN &&

[not at google - send to devlin, 2014/04/11 15:04:25]

hm is this the indentation that "git cl format" gives? dang.

ok here's an idea: can you do like "using api::tabs::ZoomSettings" or something
and cut out a lot of this "api::tabs::" stuff? then this can go on 1 line and
it'll look better. also much less visual noise elsewhere.

[paulmeyer, 2014/04/11 19:09:49]

Done. This does clean it up a lot.

+      params->zoom_settings.mode != api::tabs::ZoomSettings::MODE_AUTOMATIC &&
+      params->zoom_settings.mode != api::tabs::ZoomSettings::MODE_NONE) {
+    error_ = keys::kPerOriginOnlyInAutomaticError;
+    return false;
+  }
+
+  // Determine the correct internal zoom mode to set |web_contents| to from the
+  // user-specified |zoom_settings|.
+  content::ZoomMode zoom_mode = content::kZoomModeDefault;
+  switch (params->zoom_settings.mode) {
+    case api::tabs::ZoomSettings::MODE_NONE:
+    case api::tabs::ZoomSettings::MODE_AUTOMATIC:
+      switch (params->zoom_settings.scope) {
+        case api::tabs::ZoomSettings::SCOPE_NONE:
+        case api::tabs::ZoomSettings::SCOPE_PER_ORIGIN:
+          zoom_mode = content::kZoomModeDefault;
+          break;
+        case api::tabs::ZoomSettings::SCOPE_PER_TAB:
+          zoom_mode = content::kZoomModeIsolated;
+          break;
+      }
+      break;
+    case api::tabs::ZoomSettings::MODE_MANUAL:
+      zoom_mode = content::kZoomModeManual;
+      break;
+    case api::tabs::ZoomSettings::MODE_DISABLED:
+      zoom_mode = content::kZoomModeDisabled;
+      break;
+  }
+
+  web_contents->SetZoomMode(zoom_mode);
+
+  SendResponse(true);
+  return true;
+}
+
+bool TabsGetZoomSettingsFunction::RunImpl() {
+  scoped_ptr<tabs::GetZoomSettings::Params> params(
+      tabs::GetZoomSettings::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  WebContents* web_contents = NULL;
+  if (!GetWebContents(params->tab_id.get(), &web_contents)) {
+    return false;
+  }
+
+  content::ZoomMode zoom_mode = web_contents->GetZoomMode();
+  api::tabs::ZoomSettings zoom_settings;

[not at google - send to devlin, 2014/04/11 15:04:25]

ZoomSettings zoom_settings = ZoomModeToZoomSettings(zoom_mode);

you could also basically inline all of this stuff (into the results_ assignment)
too if you wanted.

[paulmeyer, 2014/04/11 19:09:49]

Done. I personally like separating lines when possible, especially when
combining would put them over the 80-char limit.

+  ZoomModeToZoomSettings(zoom_mode, &zoom_settings);
+
+  results_ = api::tabs::GetZoomSettings::Results::Create(zoom_settings);
+  SendResponse(true);
+  return true;
+}
+
 }  // namespace extensions