Issue 11087082: Adding progresss bar when loading fullscreen video

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+55 lines, -23 lines)			Patch
M	chrome/android/testshell/java/src/org/chromium/chrome/testshell/ChromiumTestShellActivity.java	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/android/testshell/res/values/strings.xml	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	content/browser/android/content_video_view.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	content/public/android/java/src/org/chromium/content/app/AppResource.java	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java	View	1 2 3	10 chunks	+45 lines, -22 lines	0 comments	Download
M	content/shell/android/java/src/org/chromium/content_shell/ContentShellActivity.java	View	1 3 4	3 chunks	+3 lines, -0 lines	0 comments	Download
M	content/shell/android/res/values/strings.xml	View		1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 10 (0 generated)

Expand Messages | Collapse Messages

Yaron

Seems reasonable but +Ted as I'm not well-versed in android ui http://codereview.chromium.org/11087082/diff/1/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java File content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java (left): ...

8 years, 2 months ago (2012-10-11 18:44:24 UTC) #2

qinmin

http://codereview.chromium.org/11087082/diff/1/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java File content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java (left): http://codereview.chromium.org/11087082/diff/1/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java#oldcode163 content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java:163: assert AppResource.STRING_MEDIA_PLAYER_MESSAGE_PLAYBACK_ERROR != 0; if there are no resource ...

8 years, 2 months ago (2012-10-11 18:46:17 UTC) #3

Yaron

lgtm but please wait for Ted http://codereview.chromium.org/11087082/diff/1/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java File content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java (left): http://codereview.chromium.org/11087082/diff/1/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java#oldcode163 content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java:163: assert AppResource.STRING_MEDIA_PLAYER_MESSAGE_PLAYBACK_ERROR != ...

8 years, 2 months ago (2012-10-11 18:49:27 UTC) #4

Ted C

lgtm...w/ a tiny nit http://codereview.chromium.org/11087082/diff/6001/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java File content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java (right): http://codereview.chromium.org/11087082/diff/6001/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java#newcode455 content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java:455: return (mCurrentState != STATE_ERROR && ...

8 years, 2 months ago (2012-10-15 17:43:59 UTC) #5

qinmin

http://codereview.chromium.org/11087082/diff/6001/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java File content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java (right): http://codereview.chromium.org/11087082/diff/6001/content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java#newcode455 content/public/android/java/src/org/chromium/content/browser/ContentVideoView.java:455: return (mCurrentState != STATE_ERROR && On 2012/10/15 17:43:59, Ted ...

8 years, 2 months ago (2012-10-15 18:27:40 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qinmin@chromium.org/11087082/16002

8 years, 2 months ago (2012-10-15 18:34:19 UTC) #7

commit-bot: I haz the power

Step "update" is always a major failure. Look at the try server FAQ for more ...

8 years, 2 months ago (2012-10-15 18:42:43 UTC) #8

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/qinmin@chromium.org/11087082/16002

8 years, 2 months ago (2012-10-15 20:03:20 UTC) #9

Change committed as 161969

Expand Messages | Collapse Messages

Issue 11087082: Adding progresss bar when loading fullscreen video (Closed)

Description

Patch Set 1 #

Patch Set 2 : also add the missing call to set resources for content shell #

Patch Set 3 : addressing comments #

Patch Set 4 : addressing comments #

Patch Set 5 : resolve some previous merge conflicts #

Messages