GuestView: Show status bubbles in browser

GuestView: Show status bubbles in browser

For GuestView types in the browser, such as <webview> in WebUI and
<extensionoptions> in WebUI, we would live status messages to appear
in the browser. This CL plumbs that information out to the embedder.

ContentsMouseEvent, LoadingStateChanged, and UpdateTargetURL are calls out to the content embedder that are used by Browser to update status indicators such as the spinner, and status text.

This CL plumbs those calls out from the guest to the embedder. This is a bit racy because guest status and embedder status can interleave but that's OK because these status texts are transient and eventually they'll settle and disappear so there's no long lasting raciness of consequence.

BUG=453216
TBR=lazyboy@chromium.org

Committed: https://crrev.com/67ef1b05036e92918ed09e1409abb9ce0eccecad
Cr-Commit-Position: refs/heads/master@{#314005}

[Fady Samuel, 2015-01-29 21:44:52]

fsamuel@chromium.org changed reviewers:
+ lazyboy@chromium.org, sky@chromium.org

[Fady Samuel, 2015-01-29 21:44:53]

+sky@ for core_tab_helper: Recurses into guests, if they exists on the page.
+lazyboy@ for guest_view changes. Plumbs info into embedder.

[sky, 2015-01-29 23:44:18]

https://codereview.chromium.org/880983006/diff/20001/chrome/browser/ui/tab_co...
File chrome/browser/ui/tab_contents/core_tab_helper.cc (right):

https://codereview.chromium.org/880983006/diff/20001/chrome/browser/ui/tab_co...
chrome/browser/ui/tab_contents/core_tab_helper.cc:94: source,
base::Bind(&CoreTabHelper::GetStatusTextForWebContents,
This call (and 172ish) looks expensive. How often is GetStatusTextForWebContents
invoked? Can we only do this if there is a guest_manager?

[Fady Samuel, 2015-01-30 05:43:56]

https://codereview.chromium.org/880983006/diff/20001/chrome/browser/ui/tab_co...
File chrome/browser/ui/tab_contents/core_tab_helper.cc (right):

https://codereview.chromium.org/880983006/diff/20001/chrome/browser/ui/tab_co...
chrome/browser/ui/tab_contents/core_tab_helper.cc:94: source,
base::Bind(&CoreTabHelper::GetStatusTextForWebContents,
On 2015/01/29 23:44:18, sky wrote:
> This call (and 172ish) looks expensive. How often is
GetStatusTextForWebContents
> invoked? Can we only do this if there is a guest_manager?

GuestViewManagers are created on first access. I guess we can introduce
something that checks if a GuestViewManager already exists and only takes this
path if it does.

Most often, on the drive by web, we don't run into guest WebContents and so this
call will immediately return false.

If we navigate to a PDF, then the PDF Viewer is a "guest" and so this will get
called recursively. The PDF viewer does not embed other guests.

On the signin page, the content is inside a guest.

In the extensions WebUI, the options page is a guest.

There are few guests in the wild.

Thoughts? Should I introduce a: GuestViewManager::FromBrowserContextIfAvailable
which can return nullptr?

[sky, 2015-01-30 18:50:33]

I like only doing this work if necessary. So, ya, something to see if the
guestviewmanager exists first.

On Thu, Jan 29, 2015 at 9:43 PM, <fsamuel@chromium.org> wrote:

>
> https://codereview.chromium.org/880983006/diff/20001/
> chrome/browser/ui/tab_contents/core_tab_helper.cc
> File chrome/browser/ui/tab_contents/core_tab_helper.cc (right):
>
> https://codereview.chromium.org/880983006/diff/20001/
> chrome/browser/ui/tab_contents/core_tab_helper.cc#newcode94
> chrome/browser/ui/tab_contents/core_tab_helper.cc:94: source,
> base::Bind(&CoreTabHelper::GetStatusTextForWebContents,
> On 2015/01/29 23:44:18, sky wrote:
>
>> This call (and 172ish) looks expensive. How often is
>>
> GetStatusTextForWebContents
>
>> invoked? Can we only do this if there is a guest_manager?
>>
>
> GuestViewManagers are created on first access. I guess we can introduce
> something that checks if a GuestViewManager already exists and only
> takes this path if it does.
>
> Most often, on the drive by web, we don't run into guest WebContents and
> so this call will immediately return false.
>
> If we navigate to a PDF, then the PDF Viewer is a "guest" and so this
> will get called recursively. The PDF viewer does not embed other guests.
>
> On the signin page, the content is inside a guest.
>
> In the extensions WebUI, the options page is a guest.
>
> There are few guests in the wild.
>
> Thoughts? Should I introduce a:
> GuestViewManager::FromBrowserContextIfAvailable which can return
> nullptr?
>
> https://codereview.chromium.org/880983006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2015-01-30 19:18:26]

LGTM

[Fady Samuel, 2015-01-30 21:18:03]

TBR'ing lazyboy@ who is currently on a plane and CQ'ing.

[Fady Samuel, 2015-01-30 21:18:07]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2015-01-30 21:19:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/880983006/40001

[commit-bot: I haz the power, 2015-01-30 22:09:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-01-30 22:10:57]

Patchset 3 (id:??) landed as
https://crrev.com/67ef1b05036e92918ed09e1409abb9ce0eccecad
Cr-Commit-Position: refs/heads/master@{#314005}