New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 688233 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 677540



Sign in to add a comment

[Presentation API] Implement reconnect() for 1-UA mode

Project Member Reported by zhaobin@chromium.org, Feb 3 2017

Issue description

Need to implement request.reconnect() according to spec: https://w3c.github.io/presentation-api/#reconnecting-to-a-presentation

Need to support following senarios:
1. call request.reconnect() on the same tab as request.start() (reconnect with existing controller connection);
2. call request.reconnect() on a different tab from request.start() (reconnect with existing receiver connection);
3. (optional) as miu@ mentioned in code review comments[1]: call request.reconnect() on a tab with a different profile.

[1] https://codereview.chromium.org/2471573005/diff/300001/chrome/browser/extensions/api/tab_capture/offscreen_tab.cc#newcode125
 
Owner: zhaobin@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a9e252f533a932033cd64afb3948f8393e2ae1f0

commit a9e252f533a932033cd64afb3948f8393e2ae1f0
Author: zhaobin <zhaobin@chromium.org>
Date: Mon Feb 27 22:05:20 2017

[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}

[modify] https://crrev.com/a9e252f533a932033cd64afb3948f8393e2ae1f0/chrome/browser/extensions/api/tab_capture/tab_capture_registry.cc

Cc: taku...@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb7c75d163e22589351f06fbef424f94d6e216f8

commit fb7c75d163e22589351f06fbef424f94d6e216f8
Author: zhaobin <zhaobin@chromium.org>
Date: Thu Mar 02 21:58:52 2017

[Presentation API] Use connection proxy to change connection state to 'closed'

blink connection.close()
  connection_proxy.close()
    ---> mojo call
      target_connection_proxy.OnClose() // change both sides' blink connection to 'closed'

For 1-UA, it works for both controller and receiver connection.
For 2-UA, browser_connection_proxy has no-op OnClose(), and state change is still handled by MRP.

Also change message_request_queue_ to std::deque and erase pending send message requests from message queue when closing connection.

BUG= 688233 

Review-Url: https://codereview.chromium.org/2714693002
Cr-Commit-Position: refs/heads/master@{#454391}

[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/chrome/browser/media/router/browser_presentation_connection_proxy.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/chrome/browser/media/router/browser_presentation_connection_proxy_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/browser/presentation/presentation_service_impl_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_connection_proxy.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_connection_proxy.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_connection_proxy_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_dispatcher.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_dispatcher.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_dispatcher_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/Source/modules/presentation/PresentationReceiverTest.cpp
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/public/platform/modules/presentation/WebPresentationClient.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionProxy.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/public/platform/modules/presentation/presentation.mojom

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb7c75d163e22589351f06fbef424f94d6e216f8

commit fb7c75d163e22589351f06fbef424f94d6e216f8
Author: zhaobin <zhaobin@chromium.org>
Date: Thu Mar 02 21:58:52 2017

[Presentation API] Use connection proxy to change connection state to 'closed'

blink connection.close()
  connection_proxy.close()
    ---> mojo call
      target_connection_proxy.OnClose() // change both sides' blink connection to 'closed'

For 1-UA, it works for both controller and receiver connection.
For 2-UA, browser_connection_proxy has no-op OnClose(), and state change is still handled by MRP.

Also change message_request_queue_ to std::deque and erase pending send message requests from message queue when closing connection.

BUG= 688233 

Review-Url: https://codereview.chromium.org/2714693002
Cr-Commit-Position: refs/heads/master@{#454391}

[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/chrome/browser/media/router/browser_presentation_connection_proxy.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/chrome/browser/media/router/browser_presentation_connection_proxy_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/browser/presentation/presentation_service_impl_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_connection_proxy.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_connection_proxy.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_connection_proxy_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_dispatcher.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_dispatcher.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/content/renderer/presentation/presentation_dispatcher_unittest.cc
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/Source/modules/presentation/PresentationReceiverTest.cpp
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/public/platform/modules/presentation/WebPresentationClient.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionProxy.h
[modify] https://crrev.com/fb7c75d163e22589351f06fbef424f94d6e216f8/third_party/WebKit/public/platform/modules/presentation/presentation.mojom

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f00e8b48a63cdac2e5d21fe63581bb488c3a355f

commit f00e8b48a63cdac2e5d21fe63581bb488c3a355f
Author: zhaobin <zhaobin@chromium.org>
Date: Fri Mar 17 20:10:11 2017

[Presentation API] (browser side) Implement reconnect() for 1-UA mode

If PSDImpl::JoinSession() is called for offscreeen presentation, go to OPM instead of MRP to get media route.

BUG= 688233 

Review-Url: https://codereview.chromium.org/2714783002
Cr-Commit-Position: refs/heads/master@{#457870}

[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/chrome/browser/media/router/offscreen_presentation_manager.cc
[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/chrome/browser/media/router/offscreen_presentation_manager.h
[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/chrome/browser/media/router/offscreen_presentation_manager_unittest.cc
[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/chrome/browser/media/router/presentation_service_delegate_impl.cc
[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/chrome/browser/media/router/presentation_service_delegate_impl.h
[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/chrome/browser/media/router/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/f00e8b48a63cdac2e5d21fe63581bb488c3a355f/third_party/WebKit/Source/modules/presentation/PresentationConnection.cpp

Comment 7 by mfo...@chromium.org, Mar 23 2017

Bin, can this be closed now?

Status: Fixed (was: Assigned)

Sign in to add a comment