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

Issue 15236002: WebInbandTextTrack should be deallocated by caller (Closed)

Created:
7 years, 7 months ago by Matthew Heaney (Chromium)
Modified:
7 years, 7 months ago
CC:
blink-reviews, eae+blinkwatch
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

WebInbandTextTrack should be deallocated by caller An object that inherits from WebInbandTextTrack is created on the Chromium side of the WebKit interface, and handed to the InbandTextTrackPrivateImpl (via its constructor). The InbandTextTrackPrivateImpl object currently assumes ownership of the pointer, and so the InbandTextTrackPrivateImpl object deallocates the WebInbandTextTrack object (via its destructor). However, this means that abstractions on the Chromium side of the WebKit interface can potentially outlive the abstractions on the WebKit side on which they depend. To ameliorate such lifetime issues, the responsibility for deallocating the WebInbandTextTrack object was moved to the Chromium side, hence the InbandTextTrackPrivateImpl object is no longer required to deallocate that object. Note that the this change does not have a dedicated unit test, because no code exists that actually calls this code. These changes to WebKit are a prerequisite for the change that adds support for inband text track rendering to Chrome, as enunciated here: https://chromiumcodereview.appspot.com/13419002/ The requisite infrastructure for testing all of the changes in toto will be included when that cited change is committed. BUG=230708 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=150686

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -4 lines) Patch
M Source/WebKit/chromium/src/InbandTextTrackPrivateImpl.h View 2 chunks +1 line, -2 lines 0 comments Download
M Source/WebKit/chromium/src/InbandTextTrackPrivateImpl.cpp View 1 chunk +1 line, -2 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Matthew Heaney (Chromium)
This is a small change to the WebKit side of things. The lifetime of the ...
7 years, 7 months ago (2013-05-16 22:03:41 UTC) #1
acolwell GONE FROM CHROMIUM
lgtm
7 years, 7 months ago (2013-05-17 16:27:37 UTC) #2
Matthew Heaney (Chromium)
Tentatively the final change to WebKit before landing the CL for inband text tracks. https://codereview.chromium.org/13419002/
7 years, 7 months ago (2013-05-17 22:26:05 UTC) #3
abarth-chromium
Ok... I don't feel like I have enough context to verify that this CL is ...
7 years, 7 months ago (2013-05-20 17:44:35 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/matthewjheaney@chromium.org/15236002/1
7 years, 7 months ago (2013-05-20 17:48:27 UTC) #5
commit-bot: I haz the power
7 years, 7 months ago (2013-05-20 18:46:15 UTC) #6
Message was sent while issue was closed.
Change committed as 150686

Powered by Google App Engine
This is Rietveld 408576698