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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 910983005: Decouple performance() from LocalDOMWindow (Closed)

Created:
5 years, 10 months ago by pilgrim_google
Modified:
5 years, 10 months ago
Reviewers:
dsinclair, Mike West, jsbell
CC:
arv+blink, blink-reviews, Inactive, eric.carlson_apple.com, feature-media-reviews_chromium.org, gavinp+loader_chromium.org, Nate Chapin, philipj_slow, tyoshino+watch_chromium.org, vivekg_samsung, vivekg
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Decouple performance() from LocalDOMWindow in preparation for moving performance-related code to modules/ BUG=428290 TBR=darin@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190153

Patch Set 1 #

Total comments: 4

Patch Set 2 : rebase + feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+119 lines, -30 lines) Patch
M Source/core/core.gypi View 1 2 chunks +3 lines, -0 lines 0 comments Download
M Source/core/fetch/ResourceFetcher.cpp View 1 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/frame/DOMWindow.h View 2 chunks +0 lines, -3 lines 0 comments Download
M Source/core/frame/LocalDOMWindow.h View 2 chunks +0 lines, -3 lines 0 comments Download
M Source/core/frame/LocalDOMWindow.cpp View 4 chunks +0 lines, -10 lines 0 comments Download
M Source/core/frame/RemoteDOMWindow.h View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/frame/RemoteDOMWindow.cpp View 1 chunk +0 lines, -6 lines 0 comments Download
M Source/core/frame/Window.idl View 1 chunk +0 lines, -2 lines 0 comments Download
A Source/core/timing/DOMWindowPerformance.h View 1 1 chunk +38 lines, -0 lines 0 comments Download
A Source/core/timing/DOMWindowPerformance.cpp View 1 chunk +59 lines, -0 lines 0 comments Download
A Source/core/timing/WindowPerformance.idl View 1 1 chunk +10 lines, -0 lines 0 comments Download
M Source/modules/mediasource/VideoPlaybackQuality.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M Source/modules/webmidi/MIDIOutput.cpp View 1 2 chunks +3 lines, -2 lines 0 comments Download
M Source/web/WebLocalFrameImpl.cpp View 1 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 15 (5 generated)
pilgrim_google
Please suggest other reviewers and/or redirect if necessary.
5 years, 10 months ago (2015-02-12 18:42:50 UTC) #2
Mike West
Looks pretty reasonable. One question: https://codereview.chromium.org/910983005/diff/1/Source/core/fetch/ResourceFetcher.cpp File Source/core/fetch/ResourceFetcher.cpp (right): https://codereview.chromium.org/910983005/diff/1/Source/core/fetch/ResourceFetcher.cpp#newcode204 Source/core/fetch/ResourceFetcher.cpp:204: } Nit: You don't ...
5 years, 10 months ago (2015-02-12 18:52:44 UTC) #3
dsinclair
lgtm (besides questions from mkwst@)
5 years, 10 months ago (2015-02-12 19:45:17 UTC) #4
jsbell
lgtm too (as non-OWNER, and ditto) https://codereview.chromium.org/910983005/diff/1/Source/core/timing/WindowPerformance.idl File Source/core/timing/WindowPerformance.idl (right): https://codereview.chromium.org/910983005/diff/1/Source/core/timing/WindowPerformance.idl#newcode6 Source/core/timing/WindowPerformance.idl:6: ImplementedAs=DOMWindowPerformance, Please add ...
5 years, 10 months ago (2015-02-12 20:09:06 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/910983005/20001
5 years, 10 months ago (2015-02-13 15:39:50 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: blink_presubmit on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/26571)
5 years, 10 months ago (2015-02-13 15:44:55 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/910983005/20001
5 years, 10 months ago (2015-02-13 15:49:50 UTC) #12
Mike West
LGTM (thanks for dropping `mutable`, sorry I didn't stamp the CL last night).
5 years, 10 months ago (2015-02-13 15:52:47 UTC) #13
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://src.chromium.org/viewvc/blink?view=rev&revision=190153
5 years, 10 months ago (2015-02-13 17:00:09 UTC) #14
picksi
5 years, 10 months ago (2015-02-19 12:15:11 UTC) #15
Message was sent while issue was closed. 
While perf sheriffing, this CL (along with several others) was reported by the
bisect bots as a possible cause of the regression shown here:

https://chromeperf.appspot.com/group_report?bug_id=459175

It seems unlikely to be the root cause, but please can you take a moment to
confirm that this CL is innocent! Thanks.

Powered by Google App Engine
This is Rietveld 408576698