Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2392553003:
DevTools: continue recording network activity after load (Closed)
|
Created:
4 years, 2 months ago by valih Modified:
4 years, 2 months ago Reviewers:
caseq 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, pfeldman, kozyatinskiy+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: continue recording network activity after load
BUG=569557
Committed: https://crrev.com/dd84f83120d1ea13b61d3551f6b8134dde933f5d
Cr-Commit-Position: refs/heads/master@{#423703}
Patch Set 1 : Enabling screenshots don't stop recording when page loads #
Messages
Total messages: 24 (13 generated)
valih@google.com changed reviewers: + caseq@chromium.org
Patchset #3 (id:40001) has been deleted
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
Looks mostly good modulo a couple of comments, and please fix the subject to be more descriptive. https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js (right): https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:276: this._pendingStopTimer = setTimeout(this._displayScreenshots.bind(this), 1000); Let's also clear this._pendingStopTimer when the timer fires (i.e. delete this._pendingStopTimer in the function below). https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:279: _displayScreenshots: function() let's name it differently, e.g. _stopFilmStirpRecording
The CQ bit was checked by valih@google.com
The CQ bit was unchecked by valih@google.com
https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js (right): https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:279: _displayScreenshots: function() On 2016/10/04 00:43:37, caseq wrote: > let's name it differently, e.g. _stopFilmStirpRecording Done.
On 2016/10/04 01:13:32, valih wrote: > https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js > (right): > > https://codereview.chromium.org/2392553003/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/devtools/front_end/network/NetworkPanel.js:279: > _displayScreenshots: function() > On 2016/10/04 00:43:37, caseq wrote: > > let's name it differently, e.g. _stopFilmStirpRecording > > Done. Ouch. There's a typo there -- it should be _stopFilmStripRecording. Also, let's at least delete this._pendingStopTimer, so we don't try to clear a non-existent timeout in willReloadPage. No big side effects, but just a matter of code hygiene.
Patchset #2 (id:80001) has been deleted
Patchset #1 (id:60001) has been deleted
Patchset #1 (id:100001) has been deleted
Description was changed from ========== BUGFIX # 569557 DevTools: continue recording network activity after load BUG=569557 ========== to ========== DevTools: continue recording network activity after load BUG=569557 ==========
lgtm
The CQ bit was checked by caseq@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== DevTools: continue recording network activity after load BUG=569557 ========== to ========== DevTools: continue recording network activity after load BUG=569557 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== DevTools: continue recording network activity after load BUG=569557 ========== to ========== DevTools: continue recording network activity after load BUG=569557 Committed: https://crrev.com/dd84f83120d1ea13b61d3551f6b8134dde933f5d Cr-Commit-Position: refs/heads/master@{#423703} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/dd84f83120d1ea13b61d3551f6b8134dde933f5d Cr-Commit-Position: refs/heads/master@{#423703} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
