Make command-zero reset page scale in addition to zoom level

Make command-zero reset page scale in addition to zoom level

BUG=456880
Committed: https://crrev.com/b7c1d6c4dc447a2faf7dbc7981917b482219d58b
Cr-Commit-Position: refs/heads/master@{#319658}

[ccameron, 2015-02-26 01:27:40]

ccameron@chromium.org changed reviewers:
+ avi@chromium.org

[ccameron, 2015-02-26 01:31:20]

ccameron@chromium.org changed reviewers:
+ wjmaclean@chromium.org
- avi@chromium.org

[ccameron, 2015-02-26 01:31:20]

Now that we have pinch-zoom on Mac, requests (see bug) have started coming in to
match the reset-pinch-zoom behavior on other browsers.

wjmaclean, wdyt of this approach? I was considering hooking this in with the
rest of the zoom IPCs, but that seemed to be unnecessarily complicated (this is
a very straight shot down to the web contents, and doesn't need to interact with
the zoom state).

[wjmaclean, 2015-02-26 16:10:37]

lgtm

This approach seems fine to me. It would be good to devise some way to test
this.

[ccameron, 2015-02-26 20:04:53]

ccameron@chromium.org changed reviewers:
+ avi@chromium.org, thakis@chromium.org

[ccameron, 2015-02-26 20:04:54]

Thanks! Updated the unit tests to make sure the IDC is no longer ignored.

avi@, can you do an OWNER review for content/
thakis@, can you do an OWNER review for chrome/

[Avi (use Gerrit), 2015-02-26 20:16:58]

https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_command_controller.cc (right):

https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser_command_controller.cc:1148:
command_updater_.UpdateCommandEnabled(IDC_ZOOM_NORMAL, true);
Can we have it be enabled only if _either_ there is a custom zoom or scale? If
there is no zoom or scale, can we have it disabled?

[ccameron, 2015-02-26 20:23:07]

https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_command_controller.cc (right):

https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser_command_controller.cc:1148:
command_updater_.UpdateCommandEnabled(IDC_ZOOM_NORMAL, true);
On 2015/02/26 20:16:58, Avi wrote:
> Can we have it be enabled only if _either_ there is a custom zoom or scale? If
> there is no zoom or scale, can we have it disabled?

That information is really deep the renderer and can be changed continuously
with pinch-zoom. It would take an enormous amount of IPC traffic to keep them in
sync (not to mention bug-prone plumbing). Is there a downside to having this
command be enabled?

[Avi (use Gerrit), 2015-02-26 20:24:47]

On 2015/02/26 20:23:07, ccameron wrote:
>
https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
> File chrome/browser/ui/browser_command_controller.cc (right):
> 
>
https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
> chrome/browser/ui/browser_command_controller.cc:1148:
> command_updater_.UpdateCommandEnabled(IDC_ZOOM_NORMAL, true);
> On 2015/02/26 20:16:58, Avi wrote:
> > Can we have it be enabled only if _either_ there is a custom zoom or scale?
If
> > there is no zoom or scale, can we have it disabled?
> 
> That information is really deep the renderer and can be changed continuously
> with pinch-zoom. It would take an enormous amount of IPC traffic to keep them
in
> sync (not to mention bug-prone plumbing). Is there a downside to having this
> command be enabled?

No huge downside, just from a UI perspective we are enabling an item when it
isn't applicable, so for niceness sake I would prefer it.

But having a way to reset the scale is more important. If you can figure out
disabling with no scale, that would nice, but it's definitely worth getting this
in.

LGTM

[Nico, 2015-02-26 20:35:22]

https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_command_controller.cc (right):

https://codereview.chromium.org/954923005/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser_command_controller.cc:1148:
command_updater_.UpdateCommandEnabled(IDC_ZOOM_NORMAL, true);
On 2015/02/26 20:23:06, ccameron wrote:
> On 2015/02/26 20:16:58, Avi wrote:
> > Can we have it be enabled only if _either_ there is a custom zoom or scale?
If
> > there is no zoom or scale, can we have it disabled?
> 
> That information is really deep the renderer and can be changed continuously
> with pinch-zoom. It would take an enormous amount of IPC traffic

You only need to send "is at no pinch" and "is at pinch". That shouldn't need
much traffic?

[ccameron, 2015-02-28 02:35:40]

Added the necessary plumbing and tests.

[Avi (use Gerrit), 2015-02-28 03:21:41]

Ooooooh. Sweeeet.

Thanks! LGTM

[ccameron, 2015-03-02 18:24:10]

post-weekend ping. Btw, there is a small Blink side to this:
https://codereview.chromium.org/971813002

Once the Chromium side is OKed, I'll send the Blink patch to its OWNERS (hmm,
that sounds like a thread to the patch...).

[ccameron, 2015-03-03 18:25:28]

Ping for thakis@ for chrome/ OWNER

[Nico, 2015-03-03 18:49:32]

chrome lgtm, nice.

(sorry, wasn't aware you were waiting for me)

[ccameron, 2015-03-04 19:14:58]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2015-03-04 19:14:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
wjmaclean@chromium.org, avi@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/954923005/#ps120001
(title: "Rebase")

[commit-bot: I haz the power, 2015-03-04 19:16:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954923005/120001

[commit-bot: I haz the power, 2015-03-04 19:44:22]

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

[commit-bot: I haz the power, 2015-03-04 19:44:23]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ccameron, 2015-03-04 21:39:40]

ccameron@chromium.org changed reviewers:
+ jschuh@chromium.org

[ccameron, 2015-03-04 21:39:41]

Adding jschuh for _messages OWNER stamp.

[jschuh, 2015-03-04 22:04:17]

ipc security lgtm (notes: browser->renderer & renderer->broswer boolean state
transition)

[ccameron, 2015-03-04 22:15:20]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2015-03-04 22:17:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954923005/120001

[commit-bot: I haz the power, 2015-03-05 04:46:20]

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

[commit-bot: I haz the power, 2015-03-05 04:46:22]

Try jobs failed on following builders:
  android_rel_tests_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_rel_tes...)

[ccameron, 2015-03-05 07:47:37]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2015-03-05 07:47:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954923005/120001

[commit-bot: I haz the power, 2015-03-05 13:38:45]

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

[commit-bot: I haz the power, 2015-03-05 13:38:47]

Try jobs failed on following builders:
  android_rel_tests_recipe on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_rel_tes...)

[ccameron, 2015-03-05 16:31:21]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2015-03-05 16:31:22]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2015-03-06 07:50:10]

Patchset #14 (id:250001) has been deleted

[ccameron, 2015-03-06 07:50:17]

Patchset #13 (id:220002) has been deleted

[ccameron, 2015-03-06 07:50:27]

Patchset #12 (id:220001) has been deleted

[ccameron, 2015-03-06 07:50:34]

Patchset #11 (id:200001) has been deleted

[ccameron, 2015-03-06 07:50:40]

Patchset #10 (id:180001) has been deleted

[ccameron, 2015-03-06 07:50:46]

Patchset #9 (id:160001) has been deleted

[ccameron, 2015-03-06 07:50:51]

Patchset #8 (id:140001) has been deleted

[ccameron, 2015-03-06 17:46:29]

Patchset #11 (id:330001) has been deleted

[ccameron, 2015-03-06 17:46:45]

Patchset #12 (id:370001) has been deleted

[ccameron, 2015-03-07 00:07:03]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2015-03-07 00:07:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
wjmaclean@chromium.org, thakis@chromium.org, avi@chromium.org,
jschuh@chromium.org
Link to the patchset: https://codereview.chromium.org/954923005/#ps420021
(title: "One more bit")

[ccameron, 2015-03-07 00:07:04]

The CQ bit was unchecked by ccameron@chromium.org

[ccameron, 2015-03-09 03:04:59]

Patchset #8 (id:270001) has been deleted

[ccameron, 2015-03-09 03:05:11]

Patchset #9 (id:310001) has been deleted

[ccameron, 2015-03-09 03:05:24]

Patchset #8 (id:290001) has been deleted

[ccameron, 2015-03-09 03:05:40]

Patchset #8 (id:350001) has been deleted

[ccameron, 2015-03-09 03:05:54]

Patchset #8 (id:390001) has been deleted

[ccameron, 2015-03-09 03:06:04]

Patchset #8 (id:410001) has been deleted

[ccameron, 2015-03-09 03:06:17]

Patchset #8 (id:430001) has been deleted

[ccameron, 2015-03-09 03:06:52]

Patchset #8 (id:420021) has been deleted

[ccameron, 2015-03-09 03:07:03]

Patchset #8 (id:460001) has been deleted

[ccameron, 2015-03-09 03:07:17]

Patchset #8 (id:480001) has been deleted

[ccameron, 2015-03-09 03:08:58]

Patchset #8 (id:500001) has been deleted

[ccameron, 2015-03-09 03:09:08]

Patchset #9 (id:540001) has been deleted

[ccameron, 2015-03-09 03:09:41]

Patchset #9 (id:550001) has been deleted

[ccameron, 2015-03-09 03:09:55]

Patchset #8 (id:520001) has been deleted

[ccameron, 2015-03-09 03:10:08]

Patchset #10 (id:610001) has been deleted

[ccameron, 2015-03-09 06:17:13]

Patchset #13 (id:690001) has been deleted

[ccameron, 2015-03-09 06:17:27]

Patchset #8 (id:570001) has been deleted

[ccameron, 2015-03-09 06:17:38]

Patchset #8 (id:590001) has been deleted

[ccameron, 2015-03-09 06:17:48]

Patchset #9 (id:650001) has been deleted

[ccameron, 2015-03-09 06:17:59]

Patchset #8 (id:630001) has been deleted

[ccameron, 2015-03-09 06:18:08]

Patchset #8 (id:670001) has been deleted

[ccameron, 2015-03-09 06:18:15]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2015-03-09 06:18:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
wjmaclean@chromium.org, thakis@chromium.org, avi@chromium.org,
jschuh@chromium.org
Link to the patchset: https://codereview.chromium.org/954923005/#ps710001
(title: "Simplify notification")

[commit-bot: I haz the power, 2015-03-09 06:19:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954923005/710001

[commit-bot: I haz the power, 2015-03-09 06:22:33]

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

[commit-bot: I haz the power, 2015-03-09 06:22:35]

Failed to apply the patch.

[commit-bot: I haz the power, 2015-03-09 06:22:43]

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

[ccameron, 2015-03-09 07:16:12]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2015-03-09 07:16:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
wjmaclean@chromium.org, thakis@chromium.org, avi@chromium.org,
jschuh@chromium.org
Link to the patchset: https://codereview.chromium.org/954923005/#ps730001
(title: "Rebase again again")

[commit-bot: I haz the power, 2015-03-09 07:17:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954923005/730001

[commit-bot: I haz the power, 2015-03-09 07:35:57]

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

[commit-bot: I haz the power, 2015-03-09 07:35:59]

Failed to apply the patch.

[commit-bot: I haz the power, 2015-03-09 07:36:08]

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

[ccameron, 2015-03-09 17:05:54]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2015-03-09 17:07:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954923005/730001

[commit-bot: I haz the power, 2015-03-09 17:08:35]

Committed patchset #9 (id:730001)

[commit-bot: I haz the power, 2015-03-09 17:10:13]

Patchset 9 (id:??) landed as
https://crrev.com/b7c1d6c4dc447a2faf7dbc7981917b482219d58b
Cr-Commit-Position: refs/heads/master@{#319658}