Changes to content/ to facilitate new zoom extension API

content/public/common/page_zoom.h

 // 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.
 
 #ifndef CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_
 #define CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_
 
 #include "content/common/content_export.h"
 
 namespace content {
@@ skipping 21 matching lines @@
 CONTENT_EXPORT extern const double kEpsilon;
 
 // Test if two zoom values (either zoom factors or zoom levels) should be
 // considered equal.
 CONTENT_EXPORT bool ZoomValuesEqual(double value_a, double value_b);
 
 // Converts between zoom factors and levels.
 CONTENT_EXPORT double ZoomLevelToZoomFactor(double zoom_level);
 CONTENT_EXPORT double ZoomFactorToZoomLevel(double factor);
 
+// Defines how zoom changes are handled.

[Charlie Reis, 2014/04/10 18:57:48]

This belongs at the top of the file since it affects everything else.

This should also say something about how zoom modes are changed.  It's not clear
when changes are allowed and what happens if you change.  For example, if you
start in the per-tab mode and have two a.com tabs at different zoom levels but
then go to default mode and zoom in, what happens?  Do they end up zoomed in by
the same incremental amount or to the same end value?

+// |kZoomModeDefault| results in default zoom behavior, i.e. zoom changes are
+//     handled automatically and on a per-origin basis, meaning that other
+//     tabs navigated to the same origin will also zoom.
+// |kZoomModeIsolated| results in zoom changes being handled automatically,

[Charlie Reis, 2014/04/10 18:57:48]

kZoomModePerTab would be more accurate.  Isolated has a lot of connotations that
I don't think we want to apply here.

+//     but on a per-tab basis. Tabs in this zoom mode will not be affected by

[Charlie Reis, 2014/04/10 18:57:48]

Does this continue to apply after cross-origin or even cross-process navigations
in a tab?  The comment should clarify.

+//     zoom changes in other tabs, and vice versa.
+// |kZoomModeManual| overrides the automatic handling of zoom changes. The
+//     NOTIFICATION_WEB_CONTENTS_ZOOM_CHANGE notification will still be

[jam, 2014/04/10 21:45:45]

this seems like an old comment since you switched to the observer. but more
importantly, i think this concept needs to be removed from the api per my other
comment with the link to the content api guidelines. we shouldn't have a mode
where content doesn't know anything. if a webcontents is in that mode, then
chrome layer should just not call that webcontents.

+//     dispatched, but the page will not actually be zoomed. These zoom changes
+//     can be handled manually by listening for that notification. Zooming in
+//     this mode is also on a per-tab basis.
+// |kZoomModeDisabled| disables all zooming in this tab. The tab will revert
+//     to default (100%) zoom, and all attempted zoom changes will be ignored.
+enum ZoomMode {
+  kZoomModeDefault,
+  kZoomModeIsolated,

[jam, 2014/04/10 21:45:45]

this really seems like the same concept of
HostZoomMapImpl::SetTemporaryZoomLevel, which we can promote to the public
interface (HostZoomMap).

+  kZoomModeManual,
+  kZoomModeDisabled,

[jam, 2014/04/10 21:45:45]

this appears to have the same problem as the above comment, namely it's set on a
webcontents by chrome, and used to ignore method calls from chrome afterwards.
if chrome layer wants to disable zooming on a webcontents, it shouldn't call
setzoom on it.

+};
+
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_COMMON_PAGE_ZOOM_H_