Scope and clean up uses of AccessibilityMode.

content/browser/web_contents/web_contents_impl.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 "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 154 matching lines @@
 #if BUILDFLAG(ENABLE_PLUGINS)
 #include "content/browser/media/session/pepper_playback_observer.h"
 #endif  // ENABLE_PLUGINS
 
 namespace content {
 namespace {
 
 const int kMinimumDelayBetweenLoadingUpdatesMS = 100;
 const char kDotGoogleDotCom[] = ".google.com";
 
+const AccessibilityMode kAccessibilityModeWebContent(

[dcheng, 2017/03/07 08:41:48]

constexpr

(I'm a little surprised that this doesn't trigger any warnings about potential
static initializers)

[dougt, 2017/03/07 16:50:46]

Yes, if it is a constexpr you do get an error/warning:

'AccessibilityMode' is not literal because it is not an aggregate and has no
constexpr constructors other than copy or move constructors

This is because constexpr implies that the value is known at complication time
-- but it's not in this case.

[dcheng, 2017/03/07 18:05:12]

If the constructor is constexpr as well, it should work.

+    AccessibilityMode::kWebContents | AccessibilityMode::kInlineTextBoxes |
+    AccessibilityMode::kScreenReader |
+    AccessibilityMode::kHTML);
+
 #if defined(OS_ANDROID)
 const char kWebContentsAndroidKey[] = "web_contents_android";
 #endif  // OS_ANDROID
 
 base::LazyInstance<std::vector<WebContentsImpl::CreatedCallback> >
 g_created_callbacks = LAZY_INSTANCE_INITIALIZER;
 
 void NotifyCacheOnIO(
     scoped_refptr<net::URLRequestContextGetter> request_context,
     const GURL& url,
@@ skipping 776 matching lines @@
   for (FrameTreeNode* node : frame_tree_.Nodes()) {
     UpdateAccessibilityModeOnFrame(node->current_frame_host());
     RenderFrameHost* pending_frame_host =
         node->render_manager()->pending_frame_host();
     if (pending_frame_host)
       UpdateAccessibilityModeOnFrame(pending_frame_host);
   }
 }
 
 void WebContentsImpl::AddAccessibilityMode(AccessibilityMode mode) {
-  SetAccessibilityMode(accessibility_mode_ | mode);
+  AccessibilityMode new_mode(accessibility_mode_);
+  new_mode |= mode;
+  SetAccessibilityMode(new_mode);
 }
 
 void WebContentsImpl::RequestAXTreeSnapshot(AXTreeSnapshotCallback callback) {
   // Send a request to each of the frames in parallel. Each one will return
   // an accessibility tree snapshot, and AXTreeSnapshotCombiner will combine
   // them into a single tree and call |callback| with that result, then
   // delete |combiner|.
   AXTreeSnapshotCombiner* combiner = new AXTreeSnapshotCombiner(callback);
   for (FrameTreeNode* frame_tree_node : frame_tree_.Nodes()) {
     bool is_root = frame_tree_node->parent() == nullptr;
@@ skipping 91 matching lines @@
 
   for (auto& observer : observers_)
     observer.UserAgentOverrideSet(override);
 }
 
 const std::string& WebContentsImpl::GetUserAgentOverride() const {
   return renderer_preferences_.user_agent_override;
 }
 
 void WebContentsImpl::EnableWebContentsOnlyAccessibilityMode() {
-  if (GetAccessibilityMode() != AccessibilityModeOff) {
+  if (!GetAccessibilityMode().is_mode_off()) {
     for (RenderFrameHost* rfh : GetAllFrames())
       ResetAccessibility(rfh);
   } else {
-    AddAccessibilityMode(ACCESSIBILITY_MODE_WEB_CONTENTS_ONLY);
+    AddAccessibilityMode(kAccessibilityModeWebContent);
   }
 }
 
 bool WebContentsImpl::IsWebContentsOnlyAccessibilityModeForTesting() const {
-  return accessibility_mode_ == ACCESSIBILITY_MODE_WEB_CONTENTS_ONLY;
+  return accessibility_mode_ == kAccessibilityModeWebContent;
 }
 
 bool WebContentsImpl::IsFullAccessibilityModeForTesting() const {
-  return accessibility_mode_ == ACCESSIBILITY_MODE_COMPLETE;
+  AccessibilityMode complete_mode(
+      AccessibilityMode::kNativeAPIs | AccessibilityMode::kWebContents |
+      AccessibilityMode::kInlineTextBoxes | AccessibilityMode::kScreenReader |
+      AccessibilityMode::kHTML);

[dcheng, 2017/03/07 08:41:48]

Would be nice not to have this duplicated in
https://codereview.chromium.org/2694413006/diff/140001/content/browser/androi...
as well.

Maybe just define the complete version as a constexpr in the AccessibilityMode
header and use it everywhere?

[dougt, 2017/03/07 16:50:46]

I considered doing that, but would that mean that every compilation unit that
includes this header would have a copy of these values. Probably doesn't matter
that much either way.  I sort of like it where we're very explict at the call
site on exactly what flags we want to use.  WDYT?

[dcheng, 2017/03/07 18:05:12]

See my previous comment about constexpr. Explicit-ness is nice, but when all we
want is to make sure all the flags are included, I think it's better to have it
defined in one place (next to where we define the flags)

+  return accessibility_mode_ == complete_mode;
 }
 
 const PageImportanceSignals& WebContentsImpl::GetPageImportanceSignals() const {
   return page_importance_signals_;
 }
 
 const base::string16& WebContentsImpl::GetTitle() const {
   // Transient entries take precedence. They are used for interstitial pages
   // that are shown on top of existing pages.
   NavigationEntry* entry = controller_.GetTransientEntry();
@@ skipping 4312 matching lines @@
     RenderViewHost* render_view_host = RenderViewHost::From(render_widget_host);
     if (!render_view_host)
       continue;
     render_view_host_set.insert(render_view_host);
   }
   for (RenderViewHost* render_view_host : render_view_host_set)
     render_view_host->OnWebkitPreferencesChanged();
 }
 
 }  // namespace content