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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2570733002: [Devtools] Fixed devtools altering srcset image after close (Closed)

Created:
4 years ago by allada
Modified:
4 years ago
Reviewers:
dgozman
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, kinuko+watch, kozyatinskiy+blink_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[Devtools] Fixed devtools altering srcset image after close This patch fixes a bug where devtools was causing blink to load images from into memory cache when devtools had overriden the scaleFactor it would parse srcset and possibly choose different image until cache was evicted. R=dgozman BUG=602957 Committed: https://crrev.com/b0a49063d037d1c009dbedec2bdc381537e263dc Cr-Commit-Position: refs/heads/master@{#439301}

Patch Set 1 #

Patch Set 2 : added test #

Total comments: 6

Patch Set 3 : fixes #

Total comments: 1

Patch Set 4 : fixes #

Total comments: 1

Patch Set 5 : changes #

Messages

Total messages: 28 (14 generated)
allada
PTL
4 years ago (2016-12-12 23:12:34 UTC) #1
dgozman
Great patch! https://codereview.chromium.org/2570733002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html File third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html (right): https://codereview.chromium.org/2570733002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html#newcode1 third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html:1: <html> Let's put this under inspector-protocol/emulation. https://codereview.chromium.org/2570733002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html#newcode82 ...
4 years ago (2016-12-13 20:56:11 UTC) #6
allada
Done. https://codereview.chromium.org/2570733002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html File third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html (right): https://codereview.chromium.org/2570733002/diff/20001/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html#newcode1 third_party/WebKit/LayoutTests/http/tests/inspector-protocol/device-scale-not-persistant.html:1: <html> On 2016/12/13 20:56:10, dgozman wrote: > Let's ...
4 years ago (2016-12-14 19:50:47 UTC) #7
dgozman
https://codereview.chromium.org/2570733002/diff/40001/third_party/WebKit/Source/web/WebViewImpl.cpp File third_party/WebKit/Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/2570733002/diff/40001/third_party/WebKit/Source/web/WebViewImpl.cpp#newcode3459 third_party/WebKit/Source/web/WebViewImpl.cpp:3459: memoryCache()->evictResources(); This is a strange place for evicting from ...
4 years ago (2016-12-14 20:05:39 UTC) #8
allada
PTaL
4 years ago (2016-12-15 01:17:08 UTC) #9
allada
PTaL
4 years ago (2016-12-15 01:17:08 UTC) #10
dgozman
lgtm https://codereview.chromium.org/2570733002/diff/60001/third_party/WebKit/Source/web/DevToolsEmulator.cpp File third_party/WebKit/Source/web/DevToolsEmulator.cpp (right): https://codereview.chromium.org/2570733002/diff/60001/third_party/WebKit/Source/web/DevToolsEmulator.cpp#newcode204 third_party/WebKit/Source/web/DevToolsEmulator.cpp:204: if (m_emulationParams.deviceScaleFactor != params.deviceScaleFactor) || !m_deviceMetricsEnabled
4 years ago (2016-12-15 03:08:25 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2570733002/100001
4 years ago (2016-12-15 21:11:09 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/357387)
4 years ago (2016-12-15 23:53:50 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2570733002/100001
4 years ago (2016-12-16 22:35:32 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/356456)
4 years ago (2016-12-17 01:03:04 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2570733002/100001
4 years ago (2016-12-17 01:23:19 UTC) #23
commit-bot: I haz the power
Committed patchset #5 (id:100001)
4 years ago (2016-12-17 02:52:15 UTC) #26
commit-bot: I haz the power
4 years ago (2016-12-17 02:56:43 UTC) #28
Message was sent while issue was closed. 
Patchset 5 (id:??) landed as
https://crrev.com/b0a49063d037d1c009dbedec2bdc381537e263dc
Cr-Commit-Position: refs/heads/master@{#439301}

Powered by Google App Engine
This is Rietveld 408576698