Add setting to isolate zoom changes by default.

Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

Design doc: https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I0U/edit?usp=sharing

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

[Kevin McNee, 2017-01-12 17:47:24]

Description was changed from

==========
Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

BUG=
==========

to

==========
Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Kevin McNee, 2017-01-12 17:53:33]

Description was changed from

==========
Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Kevin McNee, 2017-01-12 17:53:34]

mcnee@chromium.org changed reviewers:
+ wjmaclean@chromium.org

[Kevin McNee, 2017-01-12 17:54:20]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 17:55:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 18:35:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 18:36:00]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Kevin McNee, 2017-01-12 21:36:23]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 21:37:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 23:40:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-12 23:40:02]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-01-16 23:25:35]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 23:25:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-17 00:41:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-17 00:41:52]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-01-18 19:10:51]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 19:11:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 20:26:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-18 20:26:52]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2017-01-19 21:09:06]

Some initial thoughts ...

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/tabs/...
File chrome/browser/ui/tabs/tab_strip_model.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/tabs/...
chrome/browser/ui/tabs/tab_strip_model.cc:345:
old_contents->OnReplaced(new_contents);
Can we perhaps re-name this to something like CloneZoomSettings()? Or have the
tab-strip do the cloning (if it can see the zoom controller, it could contact it
directly?)?

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
File chrome/browser/ui/zoom/chrome_zoom_level_otr_delegate.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
chrome/browser/ui/zoom/chrome_zoom_level_otr_delegate.cc:21: if
(!parent_zoom_level_prefs_)
Is this null for Guest browsing mode? If so, perhaps include a comment to
explain that.

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
File chrome/browser/ui/zoom/chrome_zoom_level_prefs.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
chrome/browser/ui/zoom/chrome_zoom_level_prefs.cc:104: bool default_per_origin =
true;
Rename |default_per_origin| to something like |is_per_origin| to make it clear
we're not (always) using a default value here?

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:46: // TODO Should we use GetStoragePartition
with the SiteInstance?
If we have the SiteInstance, then it's preferable to get the StoragePartition
using it.

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:55: base::Unretained(this)));
Is this safe? What if the WebContents that owns us goes away while the
prefs-delegate is in the process of sending notifications? (Can that happen?)

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:393: if (default_per_origin_ || zoom_mode_ !=
ZOOM_MODE_ISOLATED)
Again, something like |is_per_origin| or |zoom_is_per_origin| might be clearer
here.

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
File components/zoom/zoom_controller.h (right):

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.h:55: // TODO Would we want to rename this?
Nit: write as "TODO(<ldap>):"

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
content/public/browser/web_contents.h:282: virtual void OnReplaced(WebContents*
new_contents) = 0;
Hmm, content/ isn't supposed to know anything about "tabs", so we'll need some
other description for this, or move the concept into chrome/.

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
File content/public/browser/web_contents_observer.h (right):

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
content/public/browser/web_contents_observer.h:409: // Invoked when a tab's
WebContents is replaced with another.
Ditto.

[Kevin McNee, 2017-01-25 16:35:51]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 16:36:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kevin McNee, 2017-01-25 16:41:53]

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
File chrome/browser/ui/zoom/chrome_zoom_level_otr_delegate.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
chrome/browser/ui/zoom/chrome_zoom_level_otr_delegate.cc:21: if
(!parent_zoom_level_prefs_)
On 2017/01/19 21:09:05, wjmaclean wrote:
> Is this null for Guest browsing mode? If so, perhaps include a comment to
> explain that.

This is no longer used.

[commit-bot: I haz the power, 2017-01-25 17:41:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-25 17:41:33]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-01-25 23:42:56]

content no longer knows about tabs. 

Unfortunately, HostZoomMap's current public interface is not enough to get the
temporary zoom level in the case where we need to use the HostZoomMap from the
previous storage partition. Hence, I've made GetTemporaryZoomLevel part of the
public interface. This is now the only change needed in content.

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/tabs/...
File chrome/browser/ui/tabs/tab_strip_model.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/tabs/...
chrome/browser/ui/tabs/tab_strip_model.cc:345:
old_contents->OnReplaced(new_contents);
On 2017/01/19 21:09:05, wjmaclean wrote:
> Can we perhaps re-name this to something like CloneZoomSettings()? Or have the
> tab-strip do the cloning (if it can see the zoom controller, it could contact
it
> directly?)?

Done.

The zoom controller is now notified directly.

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
content/public/browser/web_contents.h:282: virtual void OnReplaced(WebContents*
new_contents) = 0;
On 2017/01/19 21:09:06, wjmaclean wrote:
> Hmm, content/ isn't supposed to know anything about "tabs", so we'll need some
> other description for this, or move the concept into chrome/.

Done.

Removed.

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
File content/public/browser/web_contents_observer.h (right):

https://codereview.chromium.org/2630583002/diff/60001/content/public/browser/...
content/public/browser/web_contents_observer.h:409: // Invoked when a tab's
WebContents is replaced with another.
On 2017/01/19 21:09:06, wjmaclean wrote:
> Ditto.

Done.

Removed.

[Kevin McNee, 2017-01-25 23:44:02]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-25 23:44:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 03:06:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 03:06:56]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Kevin McNee, 2017-01-26 15:32:15]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 15:33:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 16:24:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 16:24:04]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-02-01 20:29:40]

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:55: base::Unretained(this)));
On 2017/01/19 21:09:05, wjmaclean wrote:
> Is this safe? What if the WebContents that owns us goes away while the
> prefs-delegate is in the process of sending notifications? (Can that happen?)

Good point. Once this ZoomController is destroyed our subscription will be
destroyed, so we don't have to worry about notifications after that even if it's
in the process of sending notifications.

However, if the WebContents's destructor is interrupted and a notification
occurs before the ZoomController is destroyed, then we'll be in trouble when we
access the WebContents (although our existing HostZoomMap subscription would
have the same issue).

To be safe, we could manually destroy our subscription during
WebContentsDestroyed. The WebContents still appears to be valid at that point.

[Kevin McNee, 2017-02-03 20:11:54]

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:46: // TODO Should we use GetStoragePartition
with the SiteInstance?
On 2017/01/19 21:09:05, wjmaclean wrote:
> If we have the SiteInstance, then it's preferable to get the StoragePartition
> using it.

Done.

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:55: base::Unretained(this)));
On 2017/02/01 20:29:40, Kevin McNee wrote:
> On 2017/01/19 21:09:05, wjmaclean wrote:
> > Is this safe? What if the WebContents that owns us goes away while the
> > prefs-delegate is in the process of sending notifications? (Can that
happen?)
> 
> Good point. Once this ZoomController is destroyed our subscription will be
> destroyed, so we don't have to worry about notifications after that even if
it's
> in the process of sending notifications.
> 
> However, if the WebContents's destructor is interrupted and a notification
> occurs before the ZoomController is destroyed, then we'll be in trouble when
we
> access the WebContents (although our existing HostZoomMap subscription would
> have the same issue).
> 
> To be safe, we could manually destroy our subscription during
> WebContentsDestroyed. The WebContents still appears to be valid at that point.

Done.

[Kevin McNee, 2017-02-03 20:12:32]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 20:13:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 20:50:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 20:50:21]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Kevin McNee, 2017-02-03 21:43:44]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 21:44:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 22:01:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 22:01:18]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-02-03 22:26:02]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 22:26:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Kevin McNee, 2017-02-03 22:51:34]

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
File chrome/browser/ui/zoom/chrome_zoom_level_prefs.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/chrome/browser/ui/zoom/...
chrome/browser/ui/zoom/chrome_zoom_level_prefs.cc:104: bool default_per_origin =
true;
On 2017/01/19 21:09:05, wjmaclean wrote:
> Rename |default_per_origin| to something like |is_per_origin| to make it clear
> we're not (always) using a default value here?

Done.

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.cc:393: if (default_per_origin_ || zoom_mode_ !=
ZOOM_MODE_ISOLATED)
On 2017/01/19 21:09:05, wjmaclean wrote:
> Again, something like |is_per_origin| or |zoom_is_per_origin| might be clearer
> here.

Done.

[Kevin McNee, 2017-02-03 23:17:20]

Switching to using code review comments for questions instead of using TODOs.

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
File components/zoom/zoom_controller.h (right):

https://codereview.chromium.org/2630583002/diff/60001/components/zoom/zoom_co...
components/zoom/zoom_controller.h:55: // TODO Would we want to rename this?
On 2017/01/19 21:09:05, wjmaclean wrote:
> Nit: write as "TODO(<ldap>):"

Acknowledged.

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
chrome/app/generated_resources.grd:11310: +        When you change a page's
zoom, remember the change for all pages on the same site
Here's some sample copy. Any other suggestions?

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/options/content_settings_exceptions_area.html
(right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/options/content_settings_exceptions_area.html:148: <div
id="zoom-levels-ignored-explanation"
Is this a suitable way of informing the user that the zoom level exceptions are
not applied with per-tab zooming?

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
File chrome/browser/ui/zoom/zoom_controller_browsertest.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
chrome/browser/ui/zoom/zoom_controller_browsertest.cc:350:
browser()->profile()->GetZoomLevelPrefs();
It looks like Profile::GetZoomLevelPrefs is to be removed. Should we just be
using the profile's default storage partition directly?

https://codereview.chromium.org/2630583002/diff/180001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1102: const char kPartitionZoomScopeIsPerOrigin[] =
Shall we get UMA data about the usage of this option?

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.cc:346: // per-host zoom level.
Is this acceptable behaviour?

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.h (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.h:56: ZOOM_MODE_DEFAULT,
Would we want to rename this?

[Kevin McNee, 2017-02-03 23:20:56]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 23:21:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-04 00:22:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-04 00:22:50]

Dry run: This issue passed the CQ dry run.

[wjmaclean, 2017-02-06 16:09:07]

See my answers to your questions. We should add a reviewer for the UI change.

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
chrome/app/generated_resources.grd:11310: +        When you change a page's
zoom, remember the change for all pages on the same site
On 2017/02/03 23:17:20, Kevin McNee wrote:
> Here's some sample copy. Any other suggestions?

Perhaps expand it a bit to indicate what happens when you don't check it. Also,
perhaps change "remember the change for all pages" to "propagate the change to
all tabs on the same site"?

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/options/content_settings_exceptions_area.html
(right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/options/content_settings_exceptions_area.html:148: <div
id="zoom-levels-ignored-explanation"
On 2017/02/03 23:17:20, Kevin McNee wrote:
> Is this a suitable way of informing the user that the zoom level exceptions
are
> not applied with per-tab zooming?

Not sure ... we'll need a UI reviewer for that.

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
File chrome/browser/ui/zoom/zoom_controller_browsertest.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
chrome/browser/ui/zoom/zoom_controller_browsertest.cc:350:
browser()->profile()->GetZoomLevelPrefs();
On 2017/02/03 23:17:20, Kevin McNee wrote:
> It looks like Profile::GetZoomLevelPrefs is to be removed. Should we just be
> using the profile's default storage partition directly?

Are ZoomLevelPrefs going away? (I haven't heard of this). What is the
corresponding information/state in the storage partition?

https://codereview.chromium.org/2630583002/diff/180001/chrome/common/pref_nam...
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/common/pref_nam...
chrome/common/pref_names.cc:1102: const char kPartitionZoomScopeIsPerOrigin[] =
On 2017/02/03 23:17:20, Kevin McNee wrote:
> Shall we get UMA data about the usage of this option?

Definitely!

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.cc:346: // per-host zoom level.
On 2017/02/03 23:17:20, Kevin McNee wrote:
> Is this acceptable behaviour?

I don't think I understand the comment. I would have thought if we're switching
away from per-tab mode that, on the next navigation, we would use the per-host
level, or the default if no per-host level exists. We could also do this
immediately though it might be a little jarring to the user if the current zoom
level suddenly changes.

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.h (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.h:56: ZOOM_MODE_DEFAULT,
On 2017/02/03 23:17:20, Kevin McNee wrote:
> Would we want to rename this?

Possibly. What did you have in mind? ZOOM_MODE_PER_ORIGIN ?

[Kevin McNee, 2017-02-06 18:26:12]

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
chrome/app/generated_resources.grd:11310: +        When you change a page's
zoom, remember the change for all pages on the same site
On 2017/02/06 16:09:07, wjmaclean wrote:
> On 2017/02/03 23:17:20, Kevin McNee wrote:
> > Here's some sample copy. Any other suggestions?
> 
> Perhaps expand it a bit to indicate what happens when you don't check it.
Also,
> perhaps change "remember the change for all pages" to "propagate the change to
> all tabs on the same site"?

Sure, although if the unchecked case needs its own explanation, perhaps it would
be better to use radio buttons--one for per-origin and one for per-tab. Perhaps
we can defer to a UI reviewer. Also, should we expect users to understand what
it means to "propagate" a change?

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/options/content_settings_exceptions_area.html
(right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/options/content_settings_exceptions_area.html:148: <div
id="zoom-levels-ignored-explanation"
On 2017/02/06 16:09:07, wjmaclean wrote:
> On 2017/02/03 23:17:20, Kevin McNee wrote:
> > Is this a suitable way of informing the user that the zoom level exceptions
> are
> > not applied with per-tab zooming?
> 
> Not sure ... we'll need a UI reviewer for that.

Sure. Shall I split up this CL first?

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
File chrome/browser/ui/zoom/zoom_controller_browsertest.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
chrome/browser/ui/zoom/zoom_controller_browsertest.cc:350:
browser()->profile()->GetZoomLevelPrefs();
On 2017/02/06 16:09:07, wjmaclean wrote:
> On 2017/02/03 23:17:20, Kevin McNee wrote:
> > It looks like Profile::GetZoomLevelPrefs is to be removed. Should we just be
> > using the profile's default storage partition directly?
> 
> Are ZoomLevelPrefs going away? (I haven't heard of this). What is the
> corresponding information/state in the storage partition?

See https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile.h?l=181

Since the prefs have been moved into the storage partition, this is now
essentially a convenience method equivalent to
|GetDefaultStoragePartition(profile())->GetZoomLevelDelegate()|. Is this
convenience method going to stick around or should we be using the latter?

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.cc:346: // per-host zoom level.
On 2017/02/06 16:09:07, wjmaclean wrote:
> On 2017/02/03 23:17:20, Kevin McNee wrote:
> > Is this acceptable behaviour?
> 
> I don't think I understand the comment. I would have thought if we're
switching
> away from per-tab mode that, on the next navigation, we would use the per-host
> level, or the default if no per-host level exists. We could also do this
> immediately though it might be a little jarring to the user if the current
zoom
> level suddenly changes.

The problem with deferring the change in zoom mode until the next navigation is
that the behaviour of a zoom change will be temporarily inconsistent with the
current setting. For example, if I enable per-origin zooming and then zoom a
tab, the zoom will not be propagated if I haven't yet navigated.

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.h (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.h:56: ZOOM_MODE_DEFAULT,
On 2017/02/06 16:09:07, wjmaclean wrote:
> On 2017/02/03 23:17:20, Kevin McNee wrote:
> > Would we want to rename this?
> 
> Possibly. What did you have in mind? ZOOM_MODE_PER_ORIGIN ?

Yep.

This enum is used internally (we convert between separate enums for tabs and
webview APIs) so renaming this to indicate that it's not necessarily the default
anymore shouldn't affect anything.

[wjmaclean, 2017-02-06 18:48:38]

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/app/generated_r...
chrome/app/generated_resources.grd:11310: +        When you change a page's
zoom, remember the change for all pages on the same site
On 2017/02/06 18:26:11, Kevin McNee wrote:
> On 2017/02/06 16:09:07, wjmaclean wrote:
> > On 2017/02/03 23:17:20, Kevin McNee wrote:
> > > Here's some sample copy. Any other suggestions?
> > 
> > Perhaps expand it a bit to indicate what happens when you don't check it.
> Also,
> > perhaps change "remember the change for all pages" to "propagate the change
to
> > all tabs on the same site"?
> 
> Sure, although if the unchecked case needs its own explanation, perhaps it
would
> be better to use radio buttons--one for per-origin and one for per-tab.
Perhaps
> we can defer to a UI reviewer. Also, should we expect users to understand what
> it means to "propagate" a change?

Yes, let's leave it to the UI reviewer.

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
File chrome/browser/resources/options/content_settings_exceptions_area.html
(right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/resourc...
chrome/browser/resources/options/content_settings_exceptions_area.html:148: <div
id="zoom-levels-ignored-explanation"
On 2017/02/06 18:26:11, Kevin McNee wrote:
> On 2017/02/06 16:09:07, wjmaclean wrote:
> > On 2017/02/03 23:17:20, Kevin McNee wrote:
> > > Is this a suitable way of informing the user that the zoom level
exceptions
> > are
> > > not applied with per-tab zooming?
> > 
> > Not sure ... we'll need a UI reviewer for that.
> 
> Sure. Shall I split up this CL first?

Yeah, once you have the CL mostly how you want it, you can start thinking about
splitting it up. Though might be good to get initial buy-in from the UI reviewer
first :-)

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
File chrome/browser/ui/zoom/zoom_controller_browsertest.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/chrome/browser/ui/zoom...
chrome/browser/ui/zoom/zoom_controller_browsertest.cc:350:
browser()->profile()->GetZoomLevelPrefs();
On 2017/02/06 18:26:12, Kevin McNee wrote:
> On 2017/02/06 16:09:07, wjmaclean wrote:
> > On 2017/02/03 23:17:20, Kevin McNee wrote:
> > > It looks like Profile::GetZoomLevelPrefs is to be removed. Should we just
be
> > > using the profile's default storage partition directly?
> > 
> > Are ZoomLevelPrefs going away? (I haven't heard of this). What is the
> > corresponding information/state in the storage partition?
> 
> See
https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile.h?l=181
> 
> Since the prefs have been moved into the storage partition, this is now
> essentially a convenience method equivalent to
> |GetDefaultStoragePartition(profile())->GetZoomLevelDelegate()|. Is this
> convenience method going to stick around or should we be using the latter?

Assume it will stick around for now.

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.cc (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.cc:346: // per-host zoom level.
On 2017/02/06 18:26:12, Kevin McNee wrote:
> On 2017/02/06 16:09:07, wjmaclean wrote:
> > On 2017/02/03 23:17:20, Kevin McNee wrote:
> > > Is this acceptable behaviour?
> > 
> > I don't think I understand the comment. I would have thought if we're
> switching
> > away from per-tab mode that, on the next navigation, we would use the
per-host
> > level, or the default if no per-host level exists. We could also do this
> > immediately though it might be a little jarring to the user if the current
> zoom
> > level suddenly changes.
> 
> The problem with deferring the change in zoom mode until the next navigation
is
> that the behaviour of a zoom change will be temporarily inconsistent with the
> current setting. For example, if I enable per-origin zooming and then zoom a
> tab, the zoom will not be propagated if I haven't yet navigated.

Ok. In that case I'd clarify the comment, since it's not obvious (to me at
least) what the "first ZoomController" is going to be ...

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
File components/zoom/zoom_controller.h (right):

https://codereview.chromium.org/2630583002/diff/180001/components/zoom/zoom_c...
components/zoom/zoom_controller.h:56: ZOOM_MODE_DEFAULT,
On 2017/02/06 18:26:12, Kevin McNee wrote:
> On 2017/02/06 16:09:07, wjmaclean wrote:
> > On 2017/02/03 23:17:20, Kevin McNee wrote:
> > > Would we want to rename this?
> > 
> > Possibly. What did you have in mind? ZOOM_MODE_PER_ORIGIN ?
> 
> Yep.
> 
> This enum is used internally (we convert between separate enums for tabs and
> webview APIs) so renaming this to indicate that it's not necessarily the
default
> anymore shouldn't affect anything.

Acknowledged.

[Kevin McNee, 2017-02-09 16:01:16]

Description was changed from

==========
Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Add setting to isolate zoom changes by default.

WIP DO NOT COMMIT

Design doc:
https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I...

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Kevin McNee, 2017-02-09 16:44:46]

mcnee@chromium.org changed reviewers:
+ dbeam@chromium.org

[Kevin McNee, 2017-02-09 16:59:09]

Hello, dbeam@.

We are looking to provide a setting to allow users to enable per-tab zooming.
Please refer to the design doc:
https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I...

In particular, please take a look at the "Settings Page" subsection. We'd like
to get your input on how best to present this to the user. We've shown our
current implementation in the doc. Any comments/concerns?

Also, I still have a little bit of work to do on this CL, after which I'll be
splitting this up into smaller CLs, so no need to do a complete code review at
this point.

Thanks.

[Dan Beam, 2017-02-10 04:02:11]

On 2017/02/09 16:59:09, Kevin McNee wrote:
> Hello, dbeam@.
> 
> We are looking to provide a setting to allow users to enable per-tab zooming.
> Please refer to the design doc:
>
https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I...
> 
> In particular, please take a look at the "Settings Page" subsection. We'd like
> to get your input on how best to present this to the user. We've shown our
> current implementation in the doc. Any comments/concerns?
> 
> Also, I still have a little bit of work to do on this CL, after which I'll be
> splitting this up into smaller CLs, so no need to do a complete code review at
> this point.
> 
> Thanks.

all the options code you're touching in this CL is deprecated and will be
removed soon

/settings/ is where the new, about to launch Material Design settings page lives

chrome/browser/resources/options -> chrome/browser/resources/settings
chrome/browser/ui/webui/options -> chrome/browser/ui/webui/settings

[Kevin McNee, 2017-02-14 16:18:54]

On 2017/02/10 04:02:11, Dan Beam wrote:
> On 2017/02/09 16:59:09, Kevin McNee wrote:
> > Hello, dbeam@.
> > 
> > We are looking to provide a setting to allow users to enable per-tab
zooming.
> > Please refer to the design doc:
> >
>
https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I...
> > 
> > In particular, please take a look at the "Settings Page" subsection. We'd
like
> > to get your input on how best to present this to the user. We've shown our
> > current implementation in the doc. Any comments/concerns?
> > 
> > Also, I still have a little bit of work to do on this CL, after which I'll
be
> > splitting this up into smaller CLs, so no need to do a complete code review
at
> > this point.
> > 
> > Thanks.
> 
> all the options code you're touching in this CL is deprecated and will be
> removed soon
> 
> /settings/ is where the new, about to launch Material Design settings page
lives
> 
> chrome/browser/resources/options -> chrome/browser/resources/settings
> chrome/browser/ui/webui/options -> chrome/browser/ui/webui/settings

Okay. I'll add the option to the new settings page. I have a few questions
though.

Do you have any concerns about adding this option? I'd like to start landing CLs
for the underlying implementation.

How soon will we be switching to the new settings page? Soon enough that I
shouldn't add the option to the current settings page?

I notice that the current page zoom setting is out of the advanced settings now,
which is nice to see. That said, this setting that I'm adding would probably be
more appropriate in an advanced section. I'm wondering if the Accessibility
section might be more appropriate. Any thoughts?

Thanks.

[Kevin McNee, 2017-02-15 23:39:28]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-15 23:40:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 01:44:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 01:44:14]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  closure_compilation on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[Kevin McNee, 2017-02-16 15:18:01]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-16 15:18:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 16:38:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 16:38:14]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-02-17 02:35:38]

On 2017/02/14 16:18:54, Kevin McNee wrote:
> On 2017/02/10 04:02:11, Dan Beam wrote:
> > On 2017/02/09 16:59:09, Kevin McNee wrote:
> > > Hello, dbeam@.
> > > 
> > > We are looking to provide a setting to allow users to enable per-tab
> zooming.
> > > Please refer to the design doc:
> > >
> >
>
https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I...
> > > 
> > > In particular, please take a look at the "Settings Page" subsection. We'd
> like
> > > to get your input on how best to present this to the user. We've shown our
> > > current implementation in the doc. Any comments/concerns?
> > > 
> > > Also, I still have a little bit of work to do on this CL, after which I'll
> be
> > > splitting this up into smaller CLs, so no need to do a complete code
review
> at
> > > this point.
> > > 
> > > Thanks.
> > 
> > all the options code you're touching in this CL is deprecated and will be
> > removed soon
> > 
> > /settings/ is where the new, about to launch Material Design settings page
> lives
> > 
> > chrome/browser/resources/options -> chrome/browser/resources/settings
> > chrome/browser/ui/webui/options -> chrome/browser/ui/webui/settings
> 
> Okay. I'll add the option to the new settings page. I have a few questions
> though.
> 
> Do you have any concerns about adding this option? I'd like to start landing
CLs
> for the underlying implementation.

i have no idea and haven't looked much.  i don't really see the per-origin
configuration as a problem, but understand that some users might desire this
functionality.  it's not high on my list of stuff we should do in chrome, as it
seems like it will add complexity for minimal gain.  but i'm not the ultimate
decision maker.  have you talked to a PM?

can you add BUG= to this so I can get more context?  I noticed some bugs in your
design doc with some stars, but don't see any active bug tracking this effort.

> 
> How soon will we be switching to the new settings page? Soon enough that I
> shouldn't add the option to the current settings page?

yes

> 
> I notice that the current page zoom setting is out of the advanced settings
now,
> which is nice to see. That said, this setting that I'm adding would probably
be
> more appropriate in an advanced section. I'm wondering if the Accessibility
> section might be more appropriate. Any thoughts?

i don't currently have an opinion on whether this should be in content settings
or accessibility.

> 
> Thanks.

[Kevin McNee, 2017-02-21 22:32:20]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-21 22:34:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-21 22:41:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-21 22:41:04]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Kevin McNee, 2017-02-22 15:17:43]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 15:18:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 15:20:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 15:20:42]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Kevin McNee, 2017-02-22 16:47:53]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 16:48:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 17:18:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 17:18:28]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Kevin McNee, 2017-02-22 18:48:49]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 18:50:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 19:22:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 19:22:13]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Kevin McNee, 2017-02-22 20:47:58]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 20:48:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 21:51:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 21:51:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Kevin McNee, 2017-02-22 21:56:07]

The CQ bit was checked by mcnee@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 21:56:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 22:46:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 22:46:02]

Dry run: This issue passed the CQ dry run.

[Kevin McNee, 2017-02-23 21:43:26]

On 2017/02/17 02:35:38, Dan Beam wrote:
> On 2017/02/14 16:18:54, Kevin McNee wrote:
> > On 2017/02/10 04:02:11, Dan Beam wrote:
> > > On 2017/02/09 16:59:09, Kevin McNee wrote:
> > > > Hello, dbeam@.
> > > > 
> > > > We are looking to provide a setting to allow users to enable per-tab
> > zooming.
> > > > Please refer to the design doc:
> > > >
> > >
> >
>
https://docs.google.com/document/d/1ZOrfAApAevLzTmzlQgWuAIZ7FOMH2ojxMffqMHM4I...
> > > > 
> > > > In particular, please take a look at the "Settings Page" subsection.
We'd
> > like
> > > > to get your input on how best to present this to the user. We've shown
our
> > > > current implementation in the doc. Any comments/concerns?
> > > > 
> > > > Also, I still have a little bit of work to do on this CL, after which
I'll
> > be
> > > > splitting this up into smaller CLs, so no need to do a complete code
> review
> > at
> > > > this point.
> > > > 
> > > > Thanks.
> > > 
> > > all the options code you're touching in this CL is deprecated and will be
> > > removed soon
> > > 
> > > /settings/ is where the new, about to launch Material Design settings page
> > lives
> > > 
> > > chrome/browser/resources/options -> chrome/browser/resources/settings
> > > chrome/browser/ui/webui/options -> chrome/browser/ui/webui/settings
> > 
> > Okay. I'll add the option to the new settings page. I have a few questions
> > though.
> > 
> > Do you have any concerns about adding this option? I'd like to start landing
> CLs
> > for the underlying implementation.
> 
> i have no idea and haven't looked much.  i don't really see the per-origin
> configuration as a problem, but understand that some users might desire this
> functionality.  it's not high on my list of stuff we should do in chrome, as
it
> seems like it will add complexity for minimal gain.  but i'm not the ultimate
> decision maker.  have you talked to a PM?
> 
> can you add BUG= to this so I can get more context?  I noticed some bugs in
your
> design doc with some stars, but don't see any active bug tracking this effort.
> 
> > 
> > How soon will we be switching to the new settings page? Soon enough that I
> > shouldn't add the option to the current settings page?
> 
> yes
> 
> > 
> > I notice that the current page zoom setting is out of the advanced settings
> now,
> > which is nice to see. That said, this setting that I'm adding would probably
> be
> > more appropriate in an advanced section. I'm wondering if the Accessibility
> > section might be more appropriate. Any thoughts?
> 
> i don't currently have an opinion on whether this should be in content
settings
> or accessibility.
> 
> > 
> > Thanks.

I've moved the setting into the new settings page. With the new settings page,
we can cleanly add toggles alongside content setting exception lists, so I went
ahead and did that. Now everything is in one place in the zoom level content
settings. Yay! I've updated the design doc.

We don't currently have a PM for this. We're looking around, but if you have
someone in mind that would be well suited to this, please let us know.

Thanks.

[Dan Beam, 2017-03-10 23:16:28]

fyi: I'm not actively reviewing this code until it's decided by somebody in
product that this feature is desired in Chrome