Refactor GuestView.

chrome/browser/guestview/webview/webview_guest.cc

 // Copyright 2013 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/guestview/webview/webview_guest.h"
 
 #include "base/message_loop/message_loop.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/web_request/web_request_api.h"
@@ skipping 146 matching lines @@
 
   AttachWebViewHelpers(guest_web_contents);
 }
 
 // static
 WebViewGuest* WebViewGuest::From(int embedder_process_id,
                                  int guest_instance_id) {
   GuestView* guest = GuestView::From(embedder_process_id, guest_instance_id);
   if (!guest)
     return NULL;
-  return guest->AsWebView();
+  return guest->As<WebViewGuest>();
 }
 
 // static
 WebViewGuest* WebViewGuest::FromWebContents(WebContents* contents) {
   GuestView* guest = GuestView::FromWebContents(contents);
-  return guest ? guest->AsWebView() : NULL;
+  return guest ? guest->As<WebViewGuest>() : NULL;
 }
 
 // static.
 int WebViewGuest::GetViewInstanceId(WebContents* contents) {
   WebViewGuest* guest = FromWebContents(contents);
   if (!guest)
     return guestview::kInstanceIDNone;
 
   return guest->view_instance_id();
 }
 
 // static
+GuestView::Type WebViewGuest::Type() { return GuestView::WEBVIEW; }
+
+// static
 void WebViewGuest::RecordUserInitiatedUMA(const PermissionResponseInfo& info,
                                           bool allow) {
   if (allow) {
     // Note that |allow| == true means the embedder explicitly allowed the
     // request. For some requests they might still fail. An example of such
     // scenario would be: an embedder allows geolocation request but doesn't
     // have geolocation access on its own.
     switch (info.permission_type) {
       case BROWSER_PLUGIN_PERMISSION_TYPE_DOWNLOAD:
         content::RecordAction(
@@ skipping 86 matching lines @@
     SetUserAgentOverride(user_agent_override);
   } else {
     SetUserAgentOverride("");
   }
 
   GuestView::Attach(embedder_web_contents, args);
 
   AddWebViewToExtensionRendererState();
 }
 
-GuestView::Type WebViewGuest::GetViewType() const {
-  return GuestView::WEBVIEW;
-}
-
-WebViewGuest* WebViewGuest::AsWebView() {
-  return this;
-}
-
-AdViewGuest* WebViewGuest::AsAdView() {
-  return NULL;
-}
+GuestView::Type WebViewGuest::GetViewType() const { return Type(); }

[Fady Samuel, 2014/04/16 02:47:44]

I've been thinking. We can probably move GetViewType() and Type() to a templated
base class.

GuestViewType<WebViewGuest>

What if the relationships were:

WebViewGuest derives from GuestViewType<WebViewGuest> derives from GuestView?

[Xi Han, 2014/04/16 13:33:32]

Does it worth to introduce another layer just for these two functions? It also
introduces complexity to the code.


On 2014/04/16 02:47:44, Fady Samuel wrote:

[Xi Han, 2014/04/16 17:42:19]

As discussed offline, I will introduce a template class GuestViewType and move
related functions to this class.

On 2014/04/16 02:47:44, Fady Samuel wrote:

 
 void WebViewGuest::AddMessageToConsole(int32 level,
                                        const base::string16& message,
                                        int32 line_no,
                                        const base::string16& source_id) {
   scoped_ptr<base::DictionaryValue> args(new base::DictionaryValue());
   // Log levels are from base/logging.h: LogSeverity.
   args->SetInteger(webview::kLevel, level);
   args->SetString(webview::kMessage, message);
   args->SetInteger(webview::kLine, line_no);
@@ skipping 619 matching lines @@
     const PermissionResponseCallback& callback,
     BrowserPluginPermissionType permission_type,
     bool allowed_by_default)
     : callback(callback),
       permission_type(permission_type),
       allowed_by_default(allowed_by_default) {
 }
 
 WebViewGuest::PermissionResponseInfo::~PermissionResponseInfo() {
 }