[tab_capture] Change LOG(DFATAL) to LOG(WARNING) if fail to find new parents for offscreen tab.

[tab_capture] Change LOG(DFATAL) to LOG(WARNING) if fail to find new parents for offscreen tab.

To implement reconnect() for 1-UA mode, we are going to leave offscreen tab open when render frame resets. Hit this LOG(DFATAL) if close browser window without terminating offscreen presentation.

Offscreen tab has no root window. When tab starts, FindNewParent() will attach it to the root window of an active window. When the active window closes, OnWindowRemovingFromRootWindow() gets called and tries to FindNewParent(). If the active window is the only browser window, FindNewParent() cannot find any root window for offscreen tab and hit LOG(DFATAL).

It seems ok for offscreen tab to have no root window when browser closes. Offscreen tab will gets destroyed later with extension page.

BUG=688233
Review-Url: https://codereview.chromium.org/2711223003
Cr-Commit-Position: refs/heads/master@{#453352}
Committed: https://chromium.googlesource.com/chromium/src/+/a9e252f533a932033cd64afb3948f8393e2ae1f0

[zhaobin, 2017-02-24 19:37:45]

zhaobin@chromium.org changed reviewers:
+ imcheng@chromium.org, miu@chromium.org

[miu, 2017-02-24 22:51:55]

https://codereview.chromium.org/2711223003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc (right):

https://codereview.chromium.org/2711223003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc:107:
root_window->AddChild(content_window_);
Some work has been done in content::DelegatedFrameHost, where capture now comes
directly from the Surface. This may mean we don't need this WindowAdoptionAgent
hack anymore.

So, on your desktop, can you comment out this LOC; and then run the
browser_tests to confirm whether offscreen tabs still render and capture?

  ninja -C out/dir browser_tests
  browser_tests --gtest_filter='*TabCaptureApiPixelTest*Offscreen*'

If they still work, let's simply delete the WindowAdoptionAgent class. :)

[zhaobin, 2017-02-25 00:30:44]

https://codereview.chromium.org/2711223003/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc (right):

https://codereview.chromium.org/2711223003/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc:107:
root_window->AddChild(content_window_);
On 2017/02/24 22:51:55, miu wrote:
> Some work has been done in content::DelegatedFrameHost, where capture now
comes
> directly from the Surface. This may mean we don't need this
WindowAdoptionAgent
> hack anymore.
> 
> So, on your desktop, can you comment out this LOC; and then run the
> browser_tests to confirm whether offscreen tabs still render and capture?
> 
>   ninja -C out/dir browser_tests
>   browser_tests --gtest_filter='*TabCaptureApiPixelTest*Offscreen*'
> 
> If they still work, let's simply delete the WindowAdoptionAgent class. :)

browser_tests passed if comment out this LOC. But when trying to start an
offscreen presentation, my cast device shows a green screen. It seems that
WindowAdoptionAgent is still needed.

[miu, 2017-02-25 03:49:56]

On 2017/02/25 00:30:44, zhaobin wrote:
> browser_tests passed if comment out this LOC. But when trying to start an
> offscreen presentation, my cast device shows a green screen. It seems that
> WindowAdoptionAgent is still needed.

Okay. Thanks for checking. lgtm.

[imcheng, 2017-02-27 19:56:21]

lgtm

[zhaobin, 2017-02-27 21:12:46]

The CQ bit was checked by zhaobin@chromium.org

[commit-bot: I haz the power, 2017-02-27 21:13:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-27 22:05:25]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1488229966015110, "parent_rev":
"36db3e33fd42adbff2cb0026eba1c031cdb0fc48", "commit_rev":
"a9e252f533a932033cd64afb3948f8393e2ae1f0"}

[commit-bot: I haz the power, 2017-02-27 22:06:13]

Description was changed from

==========
[tab_capture] Change LOG(DFATAL) to LOG(WARNING) if fail to find new parents for
offscreen tab.

To implement reconnect() for 1-UA mode, we are going to leave offscreen tab open
when render frame resets. Hit this LOG(DFATAL) if close browser window without
terminating offscreen presentation.

Offscreen tab has no root window. When tab starts, FindNewParent() will attach
it to the root window of an active window. When the active window closes,
OnWindowRemovingFromRootWindow() gets called and tries to FindNewParent(). If
the active window is the only browser window, FindNewParent() cannot find any
root window for offscreen tab and hit LOG(DFATAL).

It seems ok for offscreen tab to have no root window when browser closes.
Offscreen tab will gets destroyed later with extension page.

BUG=688233
==========

to

==========
[tab_capture] Change LOG(DFATAL) to LOG(WARNING) if fail to find new parents for
offscreen tab.

To implement reconnect() for 1-UA mode, we are going to leave offscreen tab open
when render frame resets. Hit this LOG(DFATAL) if close browser window without
terminating offscreen presentation.

Offscreen tab has no root window. When tab starts, FindNewParent() will attach
it to the root window of an active window. When the active window closes,
OnWindowRemovingFromRootWindow() gets called and tries to FindNewParent(). If
the active window is the only browser window, FindNewParent() cannot find any
root window for offscreen tab and hit LOG(DFATAL).

It seems ok for offscreen tab to have no root window when browser closes.
Offscreen tab will gets destroyed later with extension page.

BUG=688233

Review-Url: https://codereview.chromium.org/2711223003
Cr-Commit-Position: refs/heads/master@{#453352}
Committed:
https://chromium.googlesource.com/chromium/src/+/a9e252f533a932033cd64afb3948...
==========

[commit-bot: I haz the power, 2017-02-27 22:06:14]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/a9e252f533a932033cd64afb3948...