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

Issue 1218023013: Hoist knowledge of image_cache into widgets/basic.dart (Closed)

Created:
5 years, 5 months ago by abarth-chromium
Modified:
5 years, 5 months ago
Reviewers:
jackson, Hixie
CC:
Aaron Boodman, ben+mojo_chromium.org, darin (slow to review), gregsimon, jackson_old, mojo-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
git@github.com:domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Hoist knowledge of image_cache into widgets/basic.dart Previously, RenderImage knew about image_cache and expected to work in terms of URLs. Now RenderImage works directly with sky.Image and it's the job of the widgets system to interact with the network cache. At the widgets layer, I've factored this work into three parts: 1) A wrapper for RenderImage that works in terms of sky.Image. 2) A component that can deal with any sort of Future<sky.Image>. 3) A NetworkImage component that translates relative URLs into Future<sky.Image> using the image_cache. A future CL will add a peer to NetworkImage that gets Future<sky.Image>s from an asset bundle. R=ianh@google.com, jackson@google.com Committed: https://chromium.googlesource.com/external/mojo/+/f55c37fc136e652b84b7f3cbaed4170836d54b5c

Patch Set 1 #

Total comments: 1

Patch Set 2 : address reviewer comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -68 lines) Patch
M sky/sdk/example/widgets/container.dart View 1 chunk +1 line, -1 line 0 comments Download
M sky/sdk/lib/mojo/net/image_cache.dart View 1 chunk +11 lines, -37 lines 0 comments Download
M sky/sdk/lib/rendering/box.dart View 1 3 chunks +10 lines, -17 lines 0 comments Download
M sky/sdk/lib/widgets/basic.dart View 1 3 chunks +53 lines, -12 lines 0 comments Download
M sky/sdk/lib/widgets/icon.dart View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (1 generated)
abarth-chromium
5 years, 5 months ago (2015-07-01 20:32:00 UTC) #1
Hixie
lgtm https://codereview.chromium.org/1218023013/diff/1/sky/sdk/lib/widgets/basic.dart File sky/sdk/lib/widgets/basic.dart (right): https://codereview.chromium.org/1218023013/diff/1/sky/sdk/lib/widgets/basic.dart#newcode495 sky/sdk/lib/widgets/basic.dart:495: void _fetchImage() { fetch sounds like it hits ...
5 years, 5 months ago (2015-07-01 20:53:34 UTC) #3
jackson
lgtm
5 years, 5 months ago (2015-07-01 21:17:07 UTC) #4
abarth-chromium
5 years, 5 months ago (2015-07-01 21:41:50 UTC) #5
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
f55c37fc136e652b84b7f3cbaed4170836d54b5c (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698