Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(5)

Issue 910053002: Validate debuggee.targetId before use in chrome.debugger (Closed)

Created:
3 years, 7 months ago by robwu
Modified:
3 years, 7 months ago
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Validate debuggee.targetId before use in chrome.debugger And refactored the tests to make sure that the debugger is detached upon returning from RunAttachFunction. Previously, if the debugger unexpectedly succeeded in attaching, the method would return (because empty error != some error), causing the attached debugger to not be detached. R=rdevlin.cronin@chromium.org BUG=456841 TEST=browser_tests DebuggerApiTest.DebuggerNotAllowedOnOtherExtensionPages Committed: https://crrev.com/409bf9d6104f83c80cd85bd261784b39cab8e93e Cr-Commit-Position: refs/heads/master@{#315675}

Patch Set 1 #

Total comments: 12

Patch Set 2 : Comments #4 #

Total comments: 6

Patch Set 3 : Comments #6 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -21 lines) Patch
M chrome/browser/extensions/api/debugger/debugger_api.cc View 1 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/extensions/api/debugger/debugger_apitest.cc View 1 2 2 chunks +58 lines, -21 lines 0 comments Download

Messages

Total messages: 12 (4 generated)
robwu
Devlin: PTAL Ben, Mustafa: FYI
3 years, 7 months ago (2015-02-09 21:18:36 UTC) #3
Devlin
Thanks for fixing this, Rob. :) https://codereview.chromium.org/910053002/diff/1/chrome/browser/extensions/api/debugger/debugger_api.cc File chrome/browser/extensions/api/debugger/debugger_api.cc (right): https://codereview.chromium.org/910053002/diff/1/chrome/browser/extensions/api/debugger/debugger_api.cc#newcode533 chrome/browser/extensions/api/debugger/debugger_api.cc:533: if (PermissionsData::IsRestrictedUrl(agent_host_->GetURL(), I'm ...
3 years, 7 months ago (2015-02-09 23:11:22 UTC) #4
robwu
https://codereview.chromium.org/910053002/diff/1/chrome/browser/extensions/api/debugger/debugger_api.cc File chrome/browser/extensions/api/debugger/debugger_api.cc (right): https://codereview.chromium.org/910053002/diff/1/chrome/browser/extensions/api/debugger/debugger_api.cc#newcode533 chrome/browser/extensions/api/debugger/debugger_api.cc:533: if (PermissionsData::IsRestrictedUrl(agent_host_->GetURL(), On 2015/02/09 23:11:22, Devlin wrote: > I'm ...
3 years, 7 months ago (2015-02-10 12:56:32 UTC) #5
Devlin
lgtm https://codereview.chromium.org/910053002/diff/1/chrome/browser/extensions/api/debugger/debugger_api.cc File chrome/browser/extensions/api/debugger/debugger_api.cc (right): https://codereview.chromium.org/910053002/diff/1/chrome/browser/extensions/api/debugger/debugger_api.cc#newcode533 chrome/browser/extensions/api/debugger/debugger_api.cc:533: if (PermissionsData::IsRestrictedUrl(agent_host_->GetURL(), On 2015/02/10 12:56:31, robwu wrote: > ...
3 years, 7 months ago (2015-02-10 17:14:43 UTC) #6
robwu
https://codereview.chromium.org/910053002/diff/20001/chrome/browser/extensions/api/debugger/debugger_apitest.cc File chrome/browser/extensions/api/debugger/debugger_apitest.cc (right): https://codereview.chromium.org/910053002/diff/20001/chrome/browser/extensions/api/debugger/debugger_apitest.cc#newcode93 chrome/browser/extensions/api/debugger/debugger_apitest.cc:93: base::ListValue* targets = NULL; On 2015/02/10 17:14:42, Devlin wrote: ...
3 years, 7 months ago (2015-02-10 22:35:44 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/910053002/40001
3 years, 7 months ago (2015-02-10 22:36:41 UTC) #10
commit-bot: I haz the power
Committed patchset #3 (id:40001)
3 years, 7 months ago (2015-02-10 23:37:15 UTC) #11
commit-bot: I haz the power
3 years, 7 months ago (2015-02-10 23:37:46 UTC) #12
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/409bf9d6104f83c80cd85bd261784b39cab8e93e
Cr-Commit-Position: refs/heads/master@{#315675}

Powered by Google App Engine
This is Rietveld 408576698