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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 284183004: Create favicon_url mirror in components/favicon. (Closed)

Created:
6 years, 7 months ago by jif
Modified:
6 years, 7 months ago
Reviewers:
danakj, jif-google, blundell
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Create favicon_url mirror in components/favicon. Original files: content/public/common/favicon_url.* BUG=359586 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271085

Patch Set 1 #

Total comments: 5

Patch Set 2 : Review fixes. #

Patch Set 3 : Alphabet. #

Total comments: 2

Patch Set 4 : Review fixes 2. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+69 lines, -1 line) Patch
M components/favicon.gypi View 1 2 3 1 chunk +10 lines, -1 line 0 comments Download
M components/favicon/DEPS View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A components/favicon/core/favicon_url.h View 1 1 chunk +36 lines, -0 lines 0 comments Download
A components/favicon/core/favicon_url.cc View 1 1 chunk +21 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
jif-google
PTAL
6 years, 7 months ago (2014-05-15 15:43:02 UTC) #1
blundell
lgtm https://codereview.chromium.org/284183004/diff/1/components/favicon/DEPS File components/favicon/DEPS (right): https://codereview.chromium.org/284183004/diff/1/components/favicon/DEPS#newcode2 components/favicon/DEPS:2: "+ui/gfx", the gypfile should have deps to mirror ...
6 years, 7 months ago (2014-05-16 07:28:45 UTC) #2
jif
https://codereview.chromium.org/284183004/diff/1/components/favicon/core/favicon_url.h File components/favicon/core/favicon_url.h (right): https://codereview.chromium.org/284183004/diff/1/components/favicon/core/favicon_url.h#newcode16 components/favicon/core/favicon_url.h:16: // The favicon url from the render. On 2014/05/16 ...
6 years, 7 months ago (2014-05-16 16:29:35 UTC) #3
jif
+danakj for new DEPS on skia
6 years, 7 months ago (2014-05-16 16:34:11 UTC) #4
danakj
https://codereview.chromium.org/284183004/diff/40001/components/favicon/DEPS File components/favicon/DEPS (right): https://codereview.chromium.org/284183004/diff/40001/components/favicon/DEPS#newcode3 components/favicon/DEPS:3: "+ui/gfx", ui/gfx/geometry?
6 years, 7 months ago (2014-05-16 16:38:39 UTC) #5
jif
Fixed. https://codereview.chromium.org/284183004/diff/40001/components/favicon/DEPS File components/favicon/DEPS (right): https://codereview.chromium.org/284183004/diff/40001/components/favicon/DEPS#newcode3 components/favicon/DEPS:3: "+ui/gfx", On 2014/05/16 16:38:39, danakj wrote: > ui/gfx/geometry? ...
6 years, 7 months ago (2014-05-16 17:04:56 UTC) #6
danakj
DEPS change LGTM
6 years, 7 months ago (2014-05-16 17:06:22 UTC) #7
jif-google
The CQ bit was checked by jif@google.com
6 years, 7 months ago (2014-05-16 17:06:59 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jif@chromium.org/284183004/60001
6 years, 7 months ago (2014-05-16 17:07:39 UTC) #9
commit-bot: I haz the power
6 years, 7 months ago (2014-05-16 19:43:28 UTC) #10
Message was sent while issue was closed. 
Change committed as 271085

Powered by Google App Engine
This is Rietveld 408576698