Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(194)

Issue 1073893003: [Presentation API] Implement ondefaultsessionstart in PSImpl. (Closed)

Created:
5 years, 8 months ago by imcheng (use chromium acct)
Modified:
5 years, 8 months ago
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, chromium-reviews, darin (slow to review), darin-cc_chromium.org, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Presentation API] Implement ondefaultsessionstart in PSImpl. Added DefaultSessionStartContext for coordinating sending default session back to PresentationDispatcher. When ListenForDefaultSessionStart is called, DefaultSessionStartContext will be installed on PresentationServiceImpl. When both the default session and PresentationDispatcher's callback are available, the callback will be invoked with the session. On Reset(), if a callback is available, it will be invoked with null. Changed PresentationDispatcher to not update Blink in that case. Also, PSImpl now keeps track of the corresponding RFH's ID instead of RFH* since most of the time we only need to use the ID. Changed PresentationServiceDelegate's Add/RemoveObserver interface, since the PresentationServiceDelegate need to be able to correlate an Observer with a RFH. (at most 1 per RFH, as it stands today). Added OnDefaultPresentationStarted to PresentationServiceDelegate::Observer interface and implemented it in PresentationServiceImpl. Added tests in PresentationServiceImpl. BUG=459001 Committed: https://crrev.com/79ddfb2383dca537e481eb9e0a477267dd48bc60 Cr-Commit-Position: refs/heads/master@{#326353}

Patch Set 1 : #

Patch Set 2 : Disable ListenForDefaultSessionStart #

Patch Set 3 : Disable both #

Patch Set 4 : add back .Reset #

Patch Set 5 : Split into 2 tests #

Patch Set 6 : split test second try #

Patch Set 7 : use RunLoopFor #

Unified diffs Side-by-side diffs Delta from patch set Stats (+242 lines, -52 lines) Patch
M content/browser/presentation/presentation_service_impl.h View 1 2 3 4 5 5 chunks +38 lines, -1 line 0 comments Download
M content/browser/presentation/presentation_service_impl.cc View 12 chunks +82 lines, -30 lines 0 comments Download
M content/browser/presentation/presentation_service_impl_unittest.cc View 1 2 3 4 5 6 8 chunks +98 lines, -12 lines 0 comments Download
M content/common/presentation/presentation_service.mojom View 1 chunk +1 line, -1 line 0 comments Download
M content/public/browser/presentation_service_delegate.h View 2 chunks +19 lines, -5 lines 0 comments Download
M content/renderer/presentation/presentation_dispatcher.cc View 1 chunk +4 lines, -3 lines 0 comments Download

Messages

Total messages: 16 (7 generated)
imcheng
Avi: PTAL at content/public/browser/presentation_service_delegate.h Anton/mark: PTAL at everything. Thanks.
5 years, 8 months ago (2015-04-17 02:11:31 UTC) #3
Avi (use Gerrit)
content/public/browser/presentation_service_delegate.h lgtm
5 years, 8 months ago (2015-04-17 02:58:04 UTC) #4
imcheng
Ping for mark / Anton.
5 years, 8 months ago (2015-04-20 21:30:02 UTC) #5
whywhat
lgtm
5 years, 8 months ago (2015-04-20 22:16:36 UTC) #7
mark a. foltz
lgtm
5 years, 8 months ago (2015-04-21 21:39:24 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1073893003/180001
5 years, 8 months ago (2015-04-22 18:50:13 UTC) #13
commit-bot: I haz the power
Committed patchset #7 (id:180001)
5 years, 8 months ago (2015-04-22 19:07:26 UTC) #14
commit-bot: I haz the power
Patchset 7 (id:??) landed as https://crrev.com/79ddfb2383dca537e481eb9e0a477267dd48bc60 Cr-Commit-Position: refs/heads/master@{#326353}
5 years, 8 months ago (2015-04-22 19:09:25 UTC) #15
vmpstr
5 years, 8 months ago (2015-04-22 22:08:36 UTC) #16
Message was sent while issue was closed.
A revert of this CL (patchset #7 id:180001) has been created in
https://codereview.chromium.org/1102683002/ by vmpstr@chromium.org.

The reason for reverting is: Speculative revert to fix content_unittests
failures:
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%....

Powered by Google App Engine
This is Rietveld 408576698