Update entry page type to include error pages when appropriate.

chrome/browser/ui/zoom/zoom_controller.cc

 // 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.
 
 #include "chrome/browser/ui/zoom/zoom_controller.h"
 
 #include "chrome/browser/ui/sad_tab.h"
 #include "chrome/browser/ui/zoom/zoom_event_manager.h"
 #include "chrome/browser/ui/zoom/zoom_observer.h"
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/page_type.h"
 #include "content/public/common/page_zoom.h"
 #include "extensions/common/extension.h"
 #include "grit/theme_resources.h"
 #include "net/base/net_util.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ZoomController);
 
 ZoomController::ZoomController(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       can_show_bubble_(true),
@@ skipping 46 matching lines @@
 }
 
 bool ZoomController::SetZoomLevel(double zoom_level) {
   // An extension did not initiate this zoom change.
   return SetZoomLevelByExtension(zoom_level, NULL);
 }
 
 bool ZoomController::SetZoomLevelByExtension(
     double zoom_level,
     const scoped_refptr<const extensions::Extension>& extension) {
+  bool is_error_page =
+      web_contents()->GetController().GetLastCommittedEntry()->GetPageType() ==
+      content::PAGE_TYPE_ERROR;
   // Cannot zoom in disabled mode. Also, don't allow changing zoom level on
-  // a crashed tab.
+  // a crashed tab or an error page.

[Charlie Reis, 2014/09/24 16:43:43]

Sanity check: I realize that changing the zoom level does nothing on error pages
today, and this change makes that explicit (e.g., disabling the zoom bubble,
etc).  An alternate fix would make the error message actually zoom, which might
be better for accessibility.  Is that worth considering, or are there reasons
not to do that?

(Either way, we'll need the PAGE_TYPE_ERROR fixes, which are great regardless.)

Oh, and it looks like interstitial pages have this same issue-- zooming them
doesn't affect the interstitial but does show the bubble.  I wonder if that's
affecting the destination site if you end up proceeding through the
interstitial?

[wjmaclean, 2014/09/24 17:59:11]

Good observation ... let's do this: make error & interstitial pages not zoom
(for now), and get the UX folks involved. But we need to proceed carefully,
since when an error page is showing, zooming will affect the intended url, not
just the error page, so we would need to think about how to handle that
properly.

[Charlie Reis, 2014/09/24 18:08:52]

Sounds perfect-- thanks.

   if (zoom_mode_ == ZOOM_MODE_DISABLED ||
-      !web_contents()->GetRenderViewHost()->IsRenderViewLive())
+      !web_contents()->GetRenderViewHost()->IsRenderViewLive() ||
+      is_error_page)
     return false;
 
   // Store extension data so that |extension| can be attributed when the zoom
   // change completes. We expect that by the time this function returns that
   // any observers that require this information will have requested it.
   last_extension_ = extension;
 
   // Do not actually rescale the page in manual mode.
   if (zoom_mode_ == ZOOM_MODE_MANUAL) {
     double old_zoom_level = zoom_level_;
@@ skipping 191 matching lines @@
   } else {
     // TODO(wjmaclean) Should we consider having HostZoomMap send both old and
     // new zoom levels here?
     double zoom_level = GetZoomLevel();
     ZoomChangedEventData zoom_change_data(
         web_contents(), zoom_level, zoom_level, zoom_mode_, can_show_bubble);
     FOR_EACH_OBSERVER(
         ZoomObserver, observers_, OnZoomChanged(zoom_change_data));
   }
 }