athena: Handle WebContents created for new windows and create activities for these.

athena/content/web_activity.cc

Index: athena/content/web_activity.cc
diff --git a/athena/content/web_activity.cc b/athena/content/web_activity.cc
index 69d44f09369eeacbdc654cf172c33bb5d41a8519..ffbf373150948a345cd04b0b92a0029ff604a3f6 100644
--- a/athena/content/web_activity.cc
+++ b/athena/content/web_activity.cc
@@ -4,10 +4,13 @@
 
 #include "athena/content/web_activity.h"
 
+#include "athena/activity/public/activity_factory.h"
 #include "athena/activity/public/activity_manager.h"
 #include "athena/input/public/accelerator_manager.h"
 #include "content/public/browser/native_web_keyboard_event.h"
+#include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_delegate.h"
 #include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/focus/focus_manager.h"
@@ -126,11 +129,21 @@ class AthenaWebView : public views::WebView {
       : views::WebView(context), controller_(new WebActivityController(this)) {
     // We create the first web contents ourselves to allow us to replace it
     // later on.
-    SetWebContents(content::WebContents::Create(
+    content::WebContents* web_contents(content::WebContents::Create(
         content::WebContents::CreateParams(context)));
+    SetWebContents(web_contents);
+    web_contents->SetDelegate(this);

[flackr, 2014/07/15 21:41:05]

FYI: The change in
https://codereview.chromium.org/332443005/diff/160001/athena/content/web_acti...
made it so that the AthenaWebView was no longer the delegate of the WebContents
as the WebView WebContents creation was responsible for assigning this. It also
breaks the WebContentsFocused and EmbedsFullscreenWidget methods which assume
that the delegate owns the webcontents so I've overridden these here.

[oshima, 2014/07/16 16:51:08]

Thanks for the fix. It was actually https://codereview.chromium.org/358003002,
but I missed that in review.

[flackr, 2014/07/18 18:15:08]

Oops, pasted the wrong link.

     // TODO(skuhne): Add content observer to detect renderer crash and set
     // content status to unloaded if that happens.
   }
+
+  AthenaWebView(content::WebContents* web_contents)
+      : views::WebView(web_contents->GetBrowserContext()),
+        controller_(new WebActivityController(this)) {
+    SetWebContents(web_contents);
+    web_contents->SetDelegate(this);
+  }
+
   virtual ~AthenaWebView() {
     // |WebView| does not own the content, so we need to destroy it here.
     content::WebContents* current_contents = GetWebContents();
@@ -161,14 +174,67 @@ class AthenaWebView : public views::WebView {
     CHECK(evicted_web_contents_.get());
     content::WebContents* null_contents = GetWebContents();
     SetWebContents(evicted_web_contents_.release());
+    web_contents()->SetDelegate(this);
     delete null_contents;
   }
 
   // Check if the content got evicted.
   const bool IsContentEvicted() { return !!evicted_web_contents_.get(); }
 
- private:
-  // WebContentsDelegate:
+  // content::WebContentsDelegate:
+  virtual content::WebContents* OpenURLFromTab(
+      content::WebContents* source,
+      const content::OpenURLParams& params) OVERRIDE {
+    switch(params.disposition) {
+      case CURRENT_TAB: {
+        DCHECK(source == web_contents());
+        content::NavigationController::LoadURLParams load_url_params(
+            params.url);
+        load_url_params.referrer = params.referrer;
+        load_url_params.frame_tree_node_id = params.frame_tree_node_id;
+        load_url_params.transition_type = params.transition;
+        load_url_params.extra_headers = params.extra_headers;
+        load_url_params.should_replace_current_entry =
+            params.should_replace_current_entry;
+        load_url_params.is_renderer_initiated = params.is_renderer_initiated;
+        load_url_params.transferred_global_request_id =
+            params.transferred_global_request_id;
+        web_contents()->GetController().LoadURLWithParams(load_url_params);
+        return web_contents();
+      }
+      case NEW_FOREGROUND_TAB:

[oshima, 2014/07/16 16:51:08]

don't you have to handle NEW_BACKGROUND_TAB as well?

[flackr, 2014/07/18 18:15:08]

Done.

+      case NEW_POPUP:
+      case NEW_WINDOW: {
+        ActivityManager::Get()->AddActivity(
+            ActivityFactory::Get()->CreateWebActivity(browser_context(),
+                                                      params.url));
+        break;
+      }
+      default:
+        break;
+    }
+    // NULL is returned if the URL wasn't opened immediately.
+    return NULL;
+  }
+
+  virtual void AddNewContents(content::WebContents* source,
+                              content::WebContents* new_contents,
+                              WindowOpenDisposition disposition,
+                              const gfx::Rect& initial_pos,
+                              bool user_gesture,
+                              bool* was_blocked) OVERRIDE {
+    ActivityManager::Get()->AddActivity(
+        new WebActivity(new AthenaWebView(new_contents)));
+  }
+
+  virtual void WebContentsFocused(content::WebContents* web_contents) OVERRIDE {
+    // Overridden because WebView expects to create and own its delegate but
+    // does not because it is owned here.
+    // TODO(flackr): Fix this since changes to WebView could introduce
+    // new WebContentsDelegate overrides which also expect to own the
+    // WebContents.

[oshima, 2014/07/16 16:51:08]

Can't we just call OnWebContentsFocused()?

[flackr, 2014/07/18 18:15:08]

Done. Though I'm still not sure why we require that the webview is the owner in
the base class, perhaps just to ensure that it didn't accidentally become the
delegate of a WebContents it doesn't own?

+  }
+
   virtual bool PreHandleKeyboardEvent(
       content::WebContents* source,
       const content::NativeWebKeyboardEvent& event,
@@ -183,6 +249,16 @@ class AthenaWebView : public views::WebView {
     controller_->HandleKeyboardEvent(source, event);
   }
 
+  virtual bool EmbedsFullscreenWidget() const OVERRIDE {
+    // Overridden because WebView expects to create and own its delegate but
+    // does not because it is owned here.
+    // TODO(flackr): Fix this since changes to WebView could introduce
+    // new WebContentsDelegate overrides which also expect to own the
+    // WebContents.

[oshima, 2014/07/16 16:51:08]

Do you know why webview is checking wc_owner there?

[flackr, 2014/07/18 18:15:08]

Looking at the original CL it was always checked there. It was likely copied
from WebContentsFocused (i.e. not requested in original review
https://chromiumcodereview.appspot.com/23477051).

I've removed the DCHECK and cc'd miu.

+    return false;
+  }
+
+ private:
   scoped_ptr<WebActivityController> controller_;
 
   // If the activity got evicted, this is the web content which holds the known
@@ -200,6 +276,16 @@ WebActivity::WebActivity(content::BrowserContext* browser_context,
       current_state_(ACTIVITY_UNLOADED) {
 }
 
+WebActivity::WebActivity(AthenaWebView* web_view)
+    : browser_context_(web_view->browser_context()),
+      url_(web_view->GetWebContents()->GetURL()),
+      web_view_(web_view),
+      current_state_(ACTIVITY_UNLOADED) {
+  // Transition to state ACTIVITY_INVISIBLE to perform the same setup steps
+  // as on new activities (namely adding a WebContentsObserver).
+  SetCurrentState(ACTIVITY_INVISIBLE);
+}
+
 WebActivity::~WebActivity() {
   // It is not required to change the activity state to UNLOADED - unless we
   // would add state observers.