Use webview in web activity

Use webview in web activity

remove test windows. i don't think we need them any more.

BUG=381477
R=mukai@chromium.org
TBR=sadrul@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275560

[oshima, 2014-06-06 20:05:23]

mukai, can you take a look?

[Jun Mukai, 2014-06-06 20:17:29]

https://codereview.chromium.org/314303004/diff/60001/athena/activity/activity...
File athena/activity/activity_manager_impl.cc (right):

https://codereview.chromium.org/314303004/diff/60001/athena/activity/activity...
athena/activity/activity_manager_impl.cc:27: while (!activities_.empty())
I think this has been fixed by another CL (sorry for typo...)

https://codereview.chromium.org/314303004/diff/60001/athena/content/web_activ...
File athena/content/web_activity.cc (right):

https://codereview.chromium.org/314303004/diff/60001/athena/content/web_activ...
athena/content/web_activity.cc:32: return
base::UTF16ToUTF8(web_view_->GetWebContents()->GetTitle());
Don't we need to check web_view_?  It might be called before GetContentsView().
Or just static_cast<views::WebView*>(GetContentsView())->...

[oshima, 2014-06-06 20:55:51]

https://codereview.chromium.org/314303004/diff/60001/athena/activity/activity...
File athena/activity/activity_manager_impl.cc (right):

https://codereview.chromium.org/314303004/diff/60001/athena/activity/activity...
athena/activity/activity_manager_impl.cc:27: while (!activities_.empty())
On 2014/06/06 20:17:29, Jun Mukai wrote:
> I think this has been fixed by another CL (sorry for typo...)

I needed this in order for the test to pass. rebased.

https://codereview.chromium.org/314303004/diff/60001/athena/content/web_activ...
File athena/content/web_activity.cc (right):

https://codereview.chromium.org/314303004/diff/60001/athena/content/web_activ...
athena/content/web_activity.cc:32: return
base::UTF16ToUTF8(web_view_->GetWebContents()->GetTitle());
On 2014/06/06 20:17:29, Jun Mukai wrote:
> Don't we need to check web_view_?  It might be called before
GetContentsView().
> Or just static_cast<views::WebView*>(GetContentsView())->...

This never called before GetContentsView(), and the view created by
GetContentsView() is supposed to be added to a parent. So if someone called
GetTitle, then delete a web activity, it will cause a memory leak.

Even if we ever changed the order, we can catch such change easily, so I wont
worry about.

[Jun Mukai, 2014-06-06 20:57:04]

lgtm

https://codereview.chromium.org/314303004/diff/60001/athena/content/web_activ...
File athena/content/web_activity.cc (right):

https://codereview.chromium.org/314303004/diff/60001/athena/content/web_activ...
athena/content/web_activity.cc:32: return
base::UTF16ToUTF8(web_view_->GetWebContents()->GetTitle());
On 2014/06/06 20:55:51, oshima wrote:
> On 2014/06/06 20:17:29, Jun Mukai wrote:
> > Don't we need to check web_view_?  It might be called before
> GetContentsView().
> > Or just static_cast<views::WebView*>(GetContentsView())->...
> 
> This never called before GetContentsView(), and the view created by
> GetContentsView() is supposed to be added to a parent. So if someone called
> GetTitle, then delete a web activity, it will cause a memory leak.
> 
> Even if we ever changed the order, we can catch such change easily, so I wont
> worry about.

fair enough. thanks.

[oshima, 2014-06-06 20:59:01]

sadrul, please let me know if you have other comments. I'll address them in
separate CL.

[oshima, 2014-06-06 20:59:03]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-06-06 20:59:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/oshima@chromium.org/314303004/80001

[oshima, 2014-06-07 00:01:43]

Committed patchset #4 manually as r275560 (presubmit successful).