Bug 53252 - Split MediaControls out of RenderMedia.
Summary: Split MediaControls out of RenderMedia.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 53020
  Show dependency treegraph
 
Reported: 2011-01-27 12:11 PST by Dimitri Glazkov (Google)
Modified: 2011-01-28 10:34 PST (History)
6 users (show)

See Also:


Attachments
Patch (66.67 KB, patch)
2011-01-27 12:24 PST, Dimitri Glazkov (Google)
eric.carlson: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2011-01-27 12:11:58 PST
Split MediaControls out of RenderMedia.
Comment 1 Dimitri Glazkov (Google) 2011-01-27 12:24:35 PST
Created attachment 80356 [details]
Patch
Comment 2 Eric Carlson 2011-01-27 14:47:26 PST
Comment on attachment 80356 [details]
Patch


  BRAVO!!

View in context: https://bugs.webkit.org/attachment.cgi?id=80356&action=review

> Source/WebCore/rendering/RenderMedia.cpp:-106
> -MediaPlayer* RenderMedia::player() const
> -{
> -    return mediaElement()->player();
> -}
> -

Why did you delete this function and update the call sites to use "mediaElement()->player()".
Comment 3 Dimitri Glazkov (Google) 2011-01-27 14:49:41 PST
Comment on attachment 80356 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=80356&action=review

>> Source/WebCore/rendering/RenderMedia.cpp:-106
>> -
> 
> Why did you delete this function and update the call sites to use "mediaElement()->player()".

No reason other than it wasn't used by this class. If you'd like, I can move it to RenderVideo instead?
Comment 4 Eric Carlson 2011-01-27 14:54:33 PST
(In reply to comment #3)
> (From update of attachment 80356 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80356&action=review
> 
> >> Source/WebCore/rendering/RenderMedia.cpp:-106
> >> -
> > 
> > Why did you delete this function and update the call sites to use "mediaElement()->player()".
> 
> No reason other than it wasn't used by this class. If you'd like, I can move it to RenderVideo instead?

Not necessary, I was just curious.
Comment 5 Dimitri Glazkov (Google) 2011-01-27 20:17:42 PST
Comment on attachment 80356 [details]
Patch

Please revert if it breaks stuff.
Comment 6 WebKit Commit Bot 2011-01-27 20:23:55 PST
Comment on attachment 80356 [details]
Patch

Rejecting attachment 80356 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'ap..." exit_code: 2

Last 500 characters of output:
ile Source/WebCore/rendering/MediaControlElements.cpp
patching file Source/WebCore/rendering/RenderMedia.cpp
patching file Source/WebCore/rendering/RenderMedia.h
Hunk #3 FAILED at 62.
Hunk #4 succeeded at 77 (offset -2 lines).
1 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/rendering/RenderMedia.h.rej
patching file Source/WebCore/rendering/RenderVideo.cpp

Failed to run "[u'/Projects/CommitQueue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Carlson', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/7568372
Comment 7 Dimitri Glazkov (Google) 2011-01-28 09:11:18 PST
Committed r76950: <http://trac.webkit.org/changeset/76950>
Comment 8 WebKit Review Bot 2011-01-28 10:34:09 PST
http://trac.webkit.org/changeset/76950 might have broken Leopard Intel Release (Tests)