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..21b8bc8e1516a6b2aa6930bf269654150193c7cc 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"
 #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,35 @@ Browser* CreateBrowserWindow(const Browser::CreateParams& params,
 
 }  // namespace
 
+void ZoomModeToZoomSettings(content::ZoomMode zoom_mode,
+                            base::DictionaryValue* zoom_settings) {
+  std::string zoom_settings_mode;
+  std::string zoom_settings_scope;

[not at google - send to devlin, 2014/04/09 03:52:11]

you could use const char* for both of these and avoid copying strings. also
given the context is clearly zoom related you could just call these "mode" and
"scope".

[paulmeyer, 2014/04/11 03:01:02]

Done.

+  switch (zoom_mode) {
+    case content::kZoomModeDefault:
+      zoom_settings_mode = "automatic";
+      zoom_settings_scope = "per-origin";
+      break;
+    case content::kZoomModeIsolated:
+      zoom_settings_mode = "automatic";
+      zoom_settings_scope = "per-tab";
+      break;
+    case content::kZoomModeManual:
+      zoom_settings_mode = "manual";
+      zoom_settings_scope = "per-tab";
+      break;
+    case content::kZoomModeDisabled:
+      zoom_settings_mode = "disabled";
+      zoom_settings_scope = "per-tab";
+      break;
+    default:
+      NOTREACHED();

[not at google - send to devlin, 2014/04/09 03:52:11]

prefer not setting a default case so that you can get compile errors when cases
are left out.

you'd need NULL-checks for those pointers.

[paulmeyer, 2014/04/11 03:01:02]

Done.

+  }
+
+  zoom_settings->SetString(keys::kZoomSettingsMode, zoom_settings_mode);
+  zoom_settings->SetString(keys::kZoomSettingsScope, zoom_settings_scope);
+}
+
 // Windows ---------------------------------------------------------------------
 
 bool WindowsGetFunction::RunImpl() {
@@ -1596,8 +1627,9 @@ bool TabsReloadFunction::RunImpl() {
                     NULL,
                     &web_contents,
                     NULL,
-                    &error_))
-    return false;
+                    &error_)) {
+      return false;
+    }
   }
 
   if (web_contents->ShowingInterstitialPage()) {
@@ -1917,4 +1949,168 @@ bool TabsInsertCSSFunction::ShouldInsertCSS() const {
   return true;
 }
 
+bool ZoomAPIFunction::GetWebContents(int* tab_id,
+                                     content::WebContents** web_contents) {

[not at google - send to devlin, 2014/04/09 03:52:11]

surely this functionality is already implemented for the other API functions.
basically everything else has the behaviour to operate on the current tab if no
tab is specified.

[paulmeyer, 2014/04/11 03:01:02]

Unfortunately it is implemented but not in a share-able way at the moment, and
also every function implements this functionality slightly differently.

On 2014/04/09 03:52:11, kalman wrote:

+  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;
+      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)) {
+    SendResponse(false);
+    return false;
+  } else if (web_contents->GetURL().SchemeIs("chrome")) {

[not at google - send to devlin, 2014/04/09 03:52:11]

use kChromeUIScheme not "chrome".

[paulmeyer, 2014/04/11 03:01:02]

Done.

+    error_ = keys::kCannotZoomChromePagesError;
+    SendResponse(false);
+    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.
+  scoped_refptr<TabsSetZoomFunction> this_ref = this;

[not at google - send to devlin, 2014/04/09 03:52:11]

I don't think this makes any difference. just pass |this| into the base::Bind
call.

[paulmeyer, 2014/04/11 03:01:02]

I believe that the scoped_ptr is necessary to preserve the lifetime of
TabsSetZoomFunction. This was the recommended way to set up the callback in the
documentation in callback.h.

On 2014/04/09 03:52:11, kalman wrote:

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

I looked at the documentation in callback.h. It references patterns like

scoped_refptr<> foo = new Foo();

which isn't the same as what you have here; |this| is not being new'd, it'll
already have a refcount:

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

so you don't need to increment it here as well. The base::Bind(...this) will
hold onto a reference as well so |this| will stay alive until that callback is
destroyed.

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

Done.

+  if (!zoom_controller->SetZoomLevelByExtension(
+          zoom_level,
+          GetExtension(),
+          base::Bind(&TabsSetZoomFunction::Callback, this_ref))) {

[not at google - send to devlin, 2014/04/09 03:52:11]

you can also bind to SendReseponse directly:

base::Bind(&TabsSetZoomFunction::SendResponse, this, true)

[paulmeyer, 2014/04/11 03:01:02]

Done.

+    // Tried to zoom a tab in disabled mode.
+    error_ = keys::kCannotZoomDisabledTabError;
+    SendResponse(false);
+    return false;
+  }
+
+  return true;
+}
+
+void TabsSetZoomFunction::Callback() {
+  SendResponse(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)) {
+    SendResponse(false);
+    return false;
+  }
+
+  double zoom_level = web_contents->GetZoomLevel();
+  double zoom_factor = content::ZoomLevelToZoomFactor(zoom_level);
+  SetResult(new base::FundamentalValue(zoom_factor));

[not at google - send to devlin, 2014/04/09 03:52:11]

prefer

result_ = tabs::getZoom::Result::Create(...)

I can't remember the exact args to pass there, look in out/Debug/gen/...

[paulmeyer, 2014/04/11 03:01:02]

Done.

+  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)) {
+    SendResponse(false);
+    return false;
+  } else if (web_contents->GetURL().SchemeIs("chrome")) {
+    error_ = keys::kCannotChangeChromePageZoomSettingsError;
+    SendResponse(false);
+    return false;
+  }
+
+  // Set default mode when omitted.
+  if (params->zoom_settings.mode == api::tabs::ZoomSettings::MODE_NONE)
+    params->zoom_settings.mode = api::tabs::ZoomSettings::MODE_AUTOMATIC;
+
+  // "per-origin" scope is only available in "automatic" mode.
+  if (params->zoom_settings.scope ==
+      api::tabs::ZoomSettings::SCOPE_PER_ORIGIN &&
+      params->zoom_settings.mode != api::tabs::ZoomSettings::MODE_AUTOMATIC) {
+    error_ = keys::kPerOriginOnlyInAutomaticError;
+    SendResponse(false);
+    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_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;
+        default:

[not at google - send to devlin, 2014/04/09 03:52:11]

don't specify a default case, then you will get compile errors when you leave
the out

[paulmeyer, 2014/04/11 03:01:02]

Done.

+          NOTREACHED();
+      }
+      break;
+    case api::tabs::ZoomSettings::MODE_MANUAL:
+      zoom_mode = content::kZoomModeManual;
+      break;
+    case api::tabs::ZoomSettings::MODE_DISABLED:
+      zoom_mode = content::kZoomModeDisabled;
+      break;
+    default:

[not at google - send to devlin, 2014/04/09 03:52:11]

ditto

[paulmeyer, 2014/04/11 03:01:02]

Done.

+      NOTREACHED();
+  }
+
+  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)) {
+    SendResponse(false);
+    return false;
+  }
+
+  content::ZoomMode zoom_mode = web_contents->GetZoomMode();
+  base::DictionaryValue* zoom_settings = new base::DictionaryValue();
+  ZoomModeToZoomSettings(zoom_mode, zoom_settings);

[not at google - send to devlin, 2014/04/09 03:52:11]

my comments at the top aside - if this function is just used to set the result
of GetZoomSetings then you can use the schema generated model, which would be
better. see comment above about where to look for the actual model, but this
would reference tabs::GetZoomSettings::Result.

[paulmeyer, 2014/04/11 03:01:02]

It is also used in tabs_event_router.cc, but I improved the function already
using schema-generated stuff.

On 2014/04/09 03:52:11, kalman wrote:

+
+  SetResult(zoom_settings);
+  SendResponse(true);
+  return true;
+}
+
 }  // namespace extensions