Refactoring ContentVideoViewContextDelegate.java

content/browser/android/content_view_core_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_ANDROID_CONTENT_VIEW_CORE_IMPL_H_
 #define CONTENT_BROWSER_ANDROID_CONTENT_VIEW_CORE_IMPL_H_
 
 #include <vector>
 
 #include "base/android/jni_helper.h"
@@ skipping 14 matching lines @@
 struct WebMenuItem;
 
 namespace ui {
 class WindowAndroid;
 }
 
 namespace content {
 class RenderWidgetHostViewAndroid;
 class SyncInputEventFilter;
 
 // TODO(jrg): this is a shell.  Upstream the rest.

[joth, 2013/04/12 17:56:24]

think this can go now :-)

 class ContentViewCoreImpl : public ContentViewCore,
                             public NotificationObserver {
  public:
   static ContentViewCoreImpl* FromWebContents(WebContents* web_contents);
   ContentViewCoreImpl(JNIEnv* env,
                       jobject obj,
                       bool hardware_accelerated,
                       bool input_events_delivered_at_vsync,
                       WebContents* web_contents,
                       ui::WindowAndroid* window_android);
@@ skipping 218 matching lines @@
   void StartContentIntent(const GURL& content_url);
 
   // Shows the disambiguation popup
   // |target_rect|   --> window coordinates which |zoomed_bitmap| represents
   // |zoomed_bitmap| --> magnified image of potential touch targets
   void ShowDisambiguationPopup(
       const gfx::Rect& target_rect, const SkBitmap& zoomed_bitmap);
 
   void RequestExternalVideoSurface(int player_id);
 
+  base::android::ScopedJavaLocalRef<jobject> GetContentVideoViewClient();
+
+  // Returns the Activity context associated with ContentViewCore.
+  // Note: This context is different with base::android::GetApplicationContext().
+  base::android::ScopedJavaLocalRef<jobject> GetContext();

[joth, 2013/04/12 17:56:24]

This is misleading, as CVC can be used without an activity, so having this
getter could lead to mistakes.
(for fullscreen video, we accept that it's just going to fail if the webview is
used with a non-activity context, but I'm worried the getter here will cause new
code to make incorrect assumptions here)

With getContext on ContentVideoView pushed the policy of how to deal this up to
the embedder, and was clear that we are asking for a context specifically for
hosting the fullscreen video, whereas here it's ambiguous what this is good for.
Maybe it's OK just to comment this is the context that the CVC was created with,
which would typically be an Activity context for an onscreen view.

[michaelbai, 2013/04/18 18:21:59]

Done.

+
   // --------------------------------------------------------------------------
   // Methods called from native code
   // --------------------------------------------------------------------------
 
   gfx::Size GetPhysicalBackingSize() const;
   gfx::Size GetViewportSizeDip() const;
   gfx::Size GetViewportSizeOffsetDip() const;
   float GetOverdrawBottomHeightDip() const;
 
   InputEventAckState FilterInputEvent(const WebKit::WebInputEvent& input_event);
@@ skipping 70 matching lines @@
   scoped_ptr<SyncInputEventFilter> input_event_filter_;
 
   DISALLOW_COPY_AND_ASSIGN(ContentViewCoreImpl);
 };
 
 bool RegisterContentViewCore(JNIEnv* env);
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_ANDROID_CONTENT_VIEW_CORE_IMPL_H_