[vr] Introduce VrTabHelper

chrome/browser/android/vr_shell/vr_tab_helper.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_ANDROID_VR_SHELL_VR_TAB_HELPER_H_
+#define CHROME_BROWSER_ANDROID_VR_SHELL_VR_TAB_HELPER_H_
+
+#include "base/macros.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace vr_shell {
+
+class VrTabHelper : public content::WebContentsObserver,

[Ted C, 2017/05/11 18:32:23]

why a WebContentsObserver?  Why being a user data on web contents, you're going
to be deleted when they do so I don't see why you need this.

[Ian Vollick, 2017/05/11 19:30:10]

It's true, I don't need to be a WebContentsObserver at this point. Removed.

+                    public content::WebContentsUserData<VrTabHelper> {
+ public:
+  ~VrTabHelper() override;

[Ted C, 2017/05/11 18:32:23]

I suspect it might also be nice to have a simple static helper method for most
callers where they could do:

VrTabHelper::IsInVr(WebContents* web_contents);

Since the bulk of callers would only really care about that, we should make it
silly simple.

[Ian Vollick, 2017/05/11 19:30:10]

Actually, we were hoping to do this slightly differently to allow per-popup
notifications to be shown via the VrShell. Basically, we're adding a
HandleThisParticularPopup() function to VrTabHelper which will do the "right
thing" for that popup. This could include closing open popups, and showing the
notifications I mentioned earlier.


Please take a look at asimjour@'s cl here for an example:
https://codereview.chromium.org/2876023002


Since this change involved creating those HandleFoo functions, I'd left that to
Amir's follow-up, but I could fuse that in if you think it'd help.

[Ted C, 2017/05/11 20:25:36]

Hmm...from our other meeting, I didn't think that was the resolution we came
to.  I thought the VrTabHelper was the way to enable all the content layer
settings, but we would look at replacing TabWebContentsDelegate with one
that is specific to VR.  I think much of that would be looking at exposing
java calls for all the UI bits to avoid the C++ inheritance.

For the things that go through the delegate, I think the short term plan
was to just use this to check whether you were in VR and abort.  But the
long term plan was for it not to delegate into here to actually do any
work.

So I think instead of having those HandleXXX functions, they should just
be returning based on the boolean and we should work on finding a way for
VR to handle this more broadly without this being the UI dumping ground
for all UI features in Chrome.

+
+  bool is_in_vr() const { return is_in_vr_; }
+
+  // Called by VrShell when we enter and exit vr mode. It finds us by looking us
+  // up on the WebContents. Eventually, we will also set a number of flags here
+  // as we enter and exit vr mode (see TODO below).
+  void set_is_in_vr(bool is_in_vr) { is_in_vr_ = is_in_vr; }

[Ted C, 2017/05/11 18:32:22]

since we expect this to do more in the not distance future, I would not inline
this now and make this:  SetInVR(bool in_vr);

[Ian Vollick, 2017/05/11 19:30:10]

Done.

+
+ private:
+  friend class content::WebContentsUserData<VrTabHelper>;
+
+  explicit VrTabHelper(content::WebContents* contents);
+
+  // TODO(asimjour): once we have per-dialog flags for disabling specific
+  // content-related popups, we should hang onto a pointer to the web contents
+  // and set those flags as we enter and exit vr mode.
+  bool is_in_vr_ = false;
+
+  DISALLOW_COPY_AND_ASSIGN(VrTabHelper);
+};
+
+}  // namespace vr_shell
+
+#endif  // CHROME_BROWSER_ANDROID_VR_SHELL_VR_TAB_HELPER_H_