Raise an infobar and deny access to WebGL if a GPU reset was detected while a web page containing W…

chrome/browser/three_d_api_observer.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/three_d_api_observer.h"
+
+#include "chrome/browser/api/infobars/confirm_infobar_delegate.h"
+#include "chrome/browser/infobars/infobar_tab_helper.h"
+#include "content/public/browser/gpu_data_manager.h"
+#include "content/public/common/three_d_api_types.h"
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(ThreeDAPIObserver)
+
+// ----------------------------------------------------------------------
+// ThreeDAPIInfoBarDelegate

[Peter Kasting, 2012/11/15 21:59:59]

Tiny nit: This is somewhat unusual for a divider style -- most of the dividers
I've seen put the class name followed on the same line by hyphens.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

OK, changed.

+
+// TODO(kbr): write a "learn more" article about the issues associated
+// with client 3D APIs and GPU resets.

[Peter Kasting, 2012/11/15 21:59:59]

Does this comment really belong here in the infobar code?

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

I really want it tied closely to the code, and have expanded the comment a
little with the removal of GetLinkText().

[Peter Kasting, 2012/11/15 23:41:24]

Ah.  It makes more sense now, thanks.

+
+class ThreeDAPIInfoBarDelegate : public ConfirmInfoBarDelegate {
+ public:
+  static void Show(

[Peter Kasting, 2012/11/15 21:59:59]

This method doesn't seem to be defined or used.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Oops. Thanks for catching this. Removed.

+      content::WebContents* web_contents,
+      const GURL& url);
+
+  ThreeDAPIInfoBarDelegate(
+      InfoBarTabHelper* helper,

[Peter Kasting, 2012/11/15 21:59:59]

Nit: Please take an InfoBarService* instead of an InfoBarTabHelper*.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+      const GURL& url,
+      content::ThreeDAPIType requester);
+  virtual ~ThreeDAPIInfoBarDelegate();

[Peter Kasting, 2012/11/15 21:59:59]

Nit: Please make everything but the constructor private

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+
+  // InfoBarDelegate:

[Peter Kasting, 2012/11/15 21:59:59]

Nit: You don't directly inherit from InfoBarDelegate, so put these in with the
ConfirmInfoBarDelegate overrides, at the top of the list.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+  virtual bool EqualsDelegate(InfoBarDelegate* delegate) const OVERRIDE;
+  virtual ThreeDAPIInfoBarDelegate* AsThreeDAPIInfoBarDelegate() OVERRIDE;
+
+  // ConfirmInfoBarDelegate:
+  virtual string16 GetMessageText() const OVERRIDE;
+  virtual int GetButtons() const OVERRIDE;
+  virtual string16 GetButtonLabel(InfoBarButton button) const OVERRIDE;
+  virtual bool Accept() OVERRIDE;
+  virtual bool Cancel() OVERRIDE;
+  virtual string16 GetLinkText() const OVERRIDE;
+  virtual bool LinkClicked(WindowOpenDisposition disposition) OVERRIDE;
+  virtual gfx::Image* GetIcon() const OVERRIDE;

[Peter Kasting, 2012/11/15 21:59:59]

Nit: This should be placed between EqualsDelegate() and
AsThreeDAPIInfoBarDelegate().

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+
+ private:
+  GURL url_;
+  content::ThreeDAPIType requester_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreeDAPIInfoBarDelegate);
+};
+
+ThreeDAPIInfoBarDelegate::ThreeDAPIInfoBarDelegate(
+    InfoBarTabHelper* helper,
+    const GURL& url,
+    content::ThreeDAPIType requester)
+    : ConfirmInfoBarDelegate(helper),
+      url_(url),
+      requester_(requester) {
+}
+
+ThreeDAPIInfoBarDelegate::~ThreeDAPIInfoBarDelegate() {
+}
+
+bool ThreeDAPIInfoBarDelegate::EqualsDelegate(InfoBarDelegate* delegate) const {
+  ThreeDAPIInfoBarDelegate* three_d_delegate =
+      delegate->AsThreeDAPIInfoBarDelegate();
+  // For the time being, if a given web page is actually using both
+  // WebGL and Pepper 3D and both APIs are blocked, just leave the
+  // first infobar up. If the user selects "try again", both APIs will
+  // be unblocked and the web page reload will succeed.
+  return (three_d_delegate != NULL);
+}
+
+ThreeDAPIInfoBarDelegate*
+ThreeDAPIInfoBarDelegate::AsThreeDAPIInfoBarDelegate() {

[Peter Kasting, 2012/11/15 21:59:59]

Nit: Indent 4

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

The C++ style guide doesn't require this here; further, emacs won't auto-indent
here and there are other examples in the code base like
src/content/renderer/pepper/pepper_plugin_delegate_impl.cc which have both the
return type and method name on the first column. Leaving this as is.

+  return this;
+}
+
+string16 ThreeDAPIInfoBarDelegate::GetMessageText() const {
+  string16 api_name;
+  switch (requester_) {
+    case content::THREE_D_API_TYPE_WEBGL:
+      api_name = l10n_util::GetStringUTF16(IDS_3D_APIS_WEBGL_NAME);
+      break;
+    case content::THREE_D_API_TYPE_PEPPER_3D:
+      api_name = l10n_util::GetStringUTF16(IDS_3D_APIS_PEPPER_3D_NAME);
+      break;
+  }
+
+  return l10n_util::GetStringFUTF16(IDS_3D_APIS_BLOCKED_TEXT,
+                                    api_name);

[Peter Kasting, 2012/11/15 21:59:59]

Nit: Shorter and simpler:

  return l10n_util::GetStringFUTF16(
      IDS_3D_APIS_BLOCKED_TEXT,
      l10n_util::GetStringUTF16(
          (requester_ == content::THREE_D_API_TYPE_WEBGL) ?
          IDS_3D_APIS_WEBGL_NAME : IDS_3D_APIS_PEPPER_3D_NAME));

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

It's desired for the build to break here if another API type is added to the
enum. Leaving this as is.

[Peter Kasting, 2012/11/15 23:41:24]

Is the build guaranteed to break on all our targets?  Or should we instead
change this to "default: NOTREACHED()"?  (The Google style guide says the
compiler will warn you, but it also only has to contend with one toolchain.)

[Ken Russell (switch to Gerrit), 2012/11/16 02:26:59]

It definitely breaks with clang, so there is no way a breaking change in this
area would get into the source tree and stay there. (I ran into the build
failure on Linux earlier when I had a "last value" entry in the enum.)

+}
+
+int ThreeDAPIInfoBarDelegate::GetButtons() const {
+  return BUTTON_OK | BUTTON_CANCEL;

[Peter Kasting, 2012/11/15 21:59:59]

This is the default behavior, don't override this function.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Oops, thanks. Done.

+}
+
+string16 ThreeDAPIInfoBarDelegate::GetButtonLabel(
+    InfoBarButton button) const {
+  switch (button) {
+    case BUTTON_OK:
+      return l10n_util::GetStringUTF16(IDS_3D_APIS_BLOCKED_OK_BUTTON_LABEL);
+    case BUTTON_CANCEL:
+      return l10n_util::GetStringUTF16(
+          IDS_3D_APIS_BLOCKED_TRY_AGAIN_BUTTON_LABEL);
+    default:
+      NOTREACHED();
+      return string16();
+  }

[Peter Kasting, 2012/11/15 21:59:59]

Nit: Our infobar code explicitly assumes that BUTTON_OK and BUTTON_CANCEL are
the only choices, so we always write this function like:

  return l10n_util::GetStringUTF16((button == BUTTON_OK) ?
      IDS_3D_APIS_BLOCKED_OK_BUTTON_LABEL :
      IDS_3D_APIS_BLOCKED_TRY_AGAIN_BUTTON_LABEL);

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+}
+
+bool ThreeDAPIInfoBarDelegate::Accept() {
+  // TODO(kbr): add UMA stats.
+  return true;
+}
+
+bool ThreeDAPIInfoBarDelegate::Cancel() {
+  // TODO(kbr): add UMA stats.
+  content::GpuDataManager::GetInstance()->UnblockDomainFrom3DAPIs(url_);
+  owner()->GetWebContents()->GetController().Reload(true);
+  return true;
+}
+
+string16 ThreeDAPIInfoBarDelegate::GetLinkText() const {
+  return string16();

[Peter Kasting, 2012/11/15 21:59:59]

This is the default behavior, don't override this function.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+}
+
+bool ThreeDAPIInfoBarDelegate::LinkClicked(
+    WindowOpenDisposition disposition) {
+  return false;

[Peter Kasting, 2012/11/15 21:59:59]

You have no link, don't override this function.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+}
+
+gfx::Image* ThreeDAPIInfoBarDelegate::GetIcon() const {
+  return NULL;

[Peter Kasting, 2012/11/15 21:59:59]

This is the default behavior, don't override this function.

However, really your infobar needs to have an icon.  It is an error for infobars
to not have an icon.  Please consult the UI designers to obtain an appropriate
one.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done.

+}
+
+
+// ----------------------------------------------------------------------
+// ThreeDAPIObserver
+
+ThreeDAPIObserver::ThreeDAPIObserver(content::WebContents* web_contents)
+    : WebContentsObserver(web_contents) {}
+
+ThreeDAPIObserver::~ThreeDAPIObserver() {}
+
+void ThreeDAPIObserver::DidBlock3DAPIs(const GURL& url,
+                                       content::ThreeDAPIType requester) {
+  InfoBarTabHelper* infobar_helper =
+      InfoBarTabHelper::FromWebContents(web_contents());

[Peter Kasting, 2012/11/15 21:59:59]

Nit: Use InfoBarService* infobar_service = ... here.

[Ken Russell (switch to Gerrit), 2012/11/15 22:59:23]

Done, thanks. Also updated the #include above.

+  infobar_helper->AddInfoBar(
+      new ThreeDAPIInfoBarDelegate(infobar_helper, url, requester));
+}