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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 196383030: [VideoPlayer] dedicated video player app (Closed)

Created:
6 years, 9 months ago by yoshiki
Modified:
6 years, 9 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, nkostylev+watch_chromium.org, yoshiki+watch_chromium.org, feature-media-reviews_chromium.org, rginda+watch_chromium.org, mtomasz+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Visibility:
Public.

Description

[VideoPlayer] dedicated video player app This commit adds a dedicated video player app. This is almost copy of the embedded video player app in Files.app. The embedded video player app in Files.app will be removed in the separated patch. BUG=271811 TEST=manually tested R=asargent@chromium.org, hirono@chromium.org TBR=sky@chromium.org, mkosiba@chromium.org # TBRing for adding a resource to browser_resources.grd. # TBRing for adding a combined script to the whitelist. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=259070

Patch Set 1 #

Total comments: 16

Patch Set 2 : Adressed the comments #

Total comments: 2

Patch Set 3 : Adressed the comment #

Total comments: 2

Patch Set 4 : Adressed the comment #

Patch Set 5 : Rebased #

Patch Set 6 : Add the script file to the whitelist. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+370 lines, -9 lines) Patch
M android_webview/tools/third_party_files_whitelist.txt View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/browser_resources.grd View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/component_loader.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/component_loader.cc View 1 2 3 4 3 chunks +7 lines, -0 lines 0 comments Download
M chrome/browser/resources/component_extension_resources.grd View 1 2 3 4 1 chunk +5 lines, -3 lines 0 comments Download
M chrome/browser/resources/file_manager/foreground/js/media/media_controls.js View 3 chunks +9 lines, -4 lines 0 comments Download
A chrome/browser/resources/video_player/OWNERS View 1 chunk +1 line, -0 lines 0 comments Download
A + chrome/browser/resources/video_player/css/video_player.css View 1 1 chunk +2 lines, -2 lines 0 comments Download
A + chrome/browser/resources/video_player/images/100/error.png View 1 Binary file 0 comments Download
A + chrome/browser/resources/video_player/images/100/icon.png View 1 Binary file 0 comments Download
A + chrome/browser/resources/video_player/images/200/error.png View 1 Binary file 0 comments Download
A + chrome/browser/resources/video_player/images/200/icon.png View 1 Binary file 0 comments Download
A chrome/browser/resources/video_player/js/background.js View 1 2 1 chunk +33 lines, -0 lines 0 comments Download
A chrome/browser/resources/video_player/js/video_player.js View 1 1 chunk +217 lines, -0 lines 0 comments Download
A chrome/browser/resources/video_player/js/video_player_scripts.js View 1 chunk +25 lines, -0 lines 0 comments Download
A chrome/browser/resources/video_player/manifest.json View 1 2 3 4 1 chunk +39 lines, -0 lines 0 comments Download
A chrome/browser/resources/video_player/video_player.html View 1 2 1 chunk +28 lines, -0 lines 0 comments Download

Messages

Total messages: 30 (0 generated)
yoshiki
hirono, PTAL. Thanks.
6 years, 9 months ago (2014-03-18 13:43:57 UTC) #1
hirono
Thanks! https://codereview.chromium.org/196383030/diff/150001/chrome/browser/resources/video_player/css/video_player.css File chrome/browser/resources/video_player/css/video_player.css (right): https://codereview.chromium.org/196383030/diff/150001/chrome/browser/resources/video_player/css/video_player.css#newcode90 chrome/browser/resources/video_player/css/video_player.css:90: url('../image/100/error.png') 1x, nit: This is not change of ...
6 years, 9 months ago (2014-03-19 04:05:12 UTC) #2
yoshiki
hirono, PTAL. Thanks. https://codereview.chromium.org/196383030/diff/150001/chrome/browser/resources/video_player/css/video_player.css File chrome/browser/resources/video_player/css/video_player.css (right): https://codereview.chromium.org/196383030/diff/150001/chrome/browser/resources/video_player/css/video_player.css#newcode90 chrome/browser/resources/video_player/css/video_player.css:90: url('../image/100/error.png') 1x, On 2014/03/19 04:05:12, hirono ...
6 years, 9 months ago (2014-03-19 06:55:41 UTC) #3
hirono
https://codereview.chromium.org/196383030/diff/170001/chrome/browser/resources/component_extension_resources.grd File chrome/browser/resources/component_extension_resources.grd (right): https://codereview.chromium.org/196383030/diff/170001/chrome/browser/resources/component_extension_resources.grd#newcode91 chrome/browser/resources/component_extension_resources.grd:91: <include name="IDR_VIDEO_PLAYER_ICON_16" file="video_player/image/100/icon.png" type="BINDATA" /> image -> images ?
6 years, 9 months ago (2014-03-19 07:04:18 UTC) #4
yoshiki
Thanks. PTAL :-) https://codereview.chromium.org/196383030/diff/170001/chrome/browser/resources/component_extension_resources.grd File chrome/browser/resources/component_extension_resources.grd (right): https://codereview.chromium.org/196383030/diff/170001/chrome/browser/resources/component_extension_resources.grd#newcode91 chrome/browser/resources/component_extension_resources.grd:91: <include name="IDR_VIDEO_PLAYER_ICON_16" file="video_player/image/100/icon.png" type="BINDATA" /> On ...
6 years, 9 months ago (2014-03-19 07:37:21 UTC) #5
hirono
lgtm! Thanks!
6 years, 9 months ago (2014-03-19 07:37:48 UTC) #6
yoshiki
arv@, could you approve to use 'chrome/browser/resources/video_player' directory> asargent@, could you take a look at ...
6 years, 9 months ago (2014-03-19 08:03:39 UTC) #7
asargent_no_longer_on_chrome
lgtm w/ one nit to fix before committing https://codereview.chromium.org/196383030/diff/190001/chrome/browser/extensions/component_loader.cc File chrome/browser/extensions/component_loader.cc (right): https://codereview.chromium.org/196383030/diff/190001/chrome/browser/extensions/component_loader.cc#newcode288 chrome/browser/extensions/component_loader.cc:288: base::FilePath(FILE_PATH_LITERAL("video_player"))); ...
6 years, 9 months ago (2014-03-20 18:42:10 UTC) #8
yoshiki
Thanks! https://codereview.chromium.org/196383030/diff/190001/chrome/browser/extensions/component_loader.cc File chrome/browser/extensions/component_loader.cc (right): https://codereview.chromium.org/196383030/diff/190001/chrome/browser/extensions/component_loader.cc#newcode288 chrome/browser/extensions/component_loader.cc:288: base::FilePath(FILE_PATH_LITERAL("video_player"))); On 2014/03/20 18:42:11, Antony Sargent wrote: > ...
6 years, 9 months ago (2014-03-24 02:08:37 UTC) #9
yoshiki
The CQ bit was checked by yoshiki@chromium.org
6 years, 9 months ago (2014-03-24 07:09:52 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoshiki@chromium.org/196383030/270001
6 years, 9 months ago (2014-03-24 07:10:00 UTC) #11
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-24 09:03:23 UTC) #12
commit-bot: I haz the power
Retried try job too often on android_dbg for step(s) slave_steps http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_dbg&number=163691
6 years, 9 months ago (2014-03-24 09:03:23 UTC) #13
yoshiki
The CQ bit was checked by yoshiki@chromium.org
6 years, 9 months ago (2014-03-24 09:39:34 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoshiki@chromium.org/196383030/290001
6 years, 9 months ago (2014-03-24 09:39:45 UTC) #15
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-24 09:41:05 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: tryserver.chromium on linux_chromium_clang_dbg
6 years, 9 months ago (2014-03-24 09:41:06 UTC) #17
yoshiki
The CQ bit was checked by yoshiki@chromium.org
6 years, 9 months ago (2014-03-24 10:13:08 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoshiki@chromium.org/196383030/290001
6 years, 9 months ago (2014-03-24 10:13:12 UTC) #19
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-24 11:21:37 UTC) #20
commit-bot: I haz the power
Retried try job too often on android_dbg for step(s) slave_steps http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_dbg&number=163717
6 years, 9 months ago (2014-03-24 11:21:38 UTC) #21
yoshiki
The CQ bit was checked by yoshiki@chromium.org
6 years, 9 months ago (2014-03-24 15:36:11 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoshiki@chromium.org/196383030/290001
6 years, 9 months ago (2014-03-24 15:36:32 UTC) #23
yoshiki
The CQ bit was checked by yoshiki@chromium.org
6 years, 9 months ago (2014-03-24 16:05:37 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoshiki@chromium.org/196383030/310001
6 years, 9 months ago (2014-03-24 16:05:50 UTC) #25
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 9 months ago (2014-03-24 18:04:28 UTC) #26
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) content_browsertests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=286836
6 years, 9 months ago (2014-03-24 18:04:29 UTC) #27
yoshiki
The CQ bit was checked by yoshiki@chromium.org
6 years, 9 months ago (2014-03-25 00:25:40 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yoshiki@chromium.org/196383030/310001
6 years, 9 months ago (2014-03-25 00:27:07 UTC) #29
yoshiki
6 years, 9 months ago (2014-03-25 01:07:11 UTC) #30
Message was sent while issue was closed. 
Committed patchset #6 manually as r259070 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698