Issue 23724026: BrowserEventRouter -> TabsEventRouter

Issue 23724026: BrowserEventRouter -> TabsEventRouter (Closed)

Created:
7 years, 3 months ago by Devlin

Modified:
7 years, 3 months ago

Reviewers:
Yoyo Zhou

CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, miu+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Visibility:
Public.

More Reviews

Description

BrowserEventRouter -> TabsEventRouter Move (and rename) c/b/e/browser_event_router -> c/b/e/api/tabs/tabs_event_router Lazily initialize TabsEventRouter Kill ExtensionService::InitEventRouters BUG=165209 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=222660

Patch Set 1 : #

Total comments: 5

Patch Set 2 : Yoyo's nits #

Created: 7 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+230 lines, -928 lines)			Patch
M	chrome/browser/extensions/api/tab_capture/tab_capture_api.cc	View		1 chunk	+0 lines, -1 line	0 comments	Download
A +	chrome/browser/extensions/api/tabs/tabs_event_router.h	View	1	5 chunks	+15 lines, -18 lines	0 comments	Download
A +	chrome/browser/extensions/api/tabs/tabs_event_router.cc	View	1	22 chunks	+167 lines, -151 lines	0 comments	Download
M	chrome/browser/extensions/api/tabs/tabs_windows_api.h	View		3 chunks	+4 lines, -2 lines	0 comments	Download
M	chrome/browser/extensions/api/tabs/tabs_windows_api.cc	View		3 chunks	+41 lines, -8 lines	0 comments	Download
D	chrome/browser/extensions/browser_event_router.h	View		1 chunk	+0 lines, -172 lines	0 comments	Download
D	chrome/browser/extensions/browser_event_router.cc	View		1 chunk	+0 lines, -537 lines	0 comments	Download
M	chrome/browser/extensions/extension_service.h	View		3 chunks	+0 lines, -14 lines	0 comments	Download
M	chrome/browser/extensions/extension_service.cc	View		3 chunks	+1 line, -12 lines	0 comments	Download
M	chrome/browser/extensions/extension_system.cc	View		1 chunk	+0 lines, -10 lines	0 comments	Download
M	chrome/chrome_browser_extensions.gypi	View	1	3 chunks	+2 lines, -3 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Devlin

https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_windows_api.cc File chrome/browser/extensions/api/tabs/tabs_windows_api.cc (right): https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_windows_api.cc#newcode86 chrome/browser/extensions/api/tabs/tabs_windows_api.cc:86: tabs_event_router(); If we *really* wanted to, we could separate ...

7 years, 3 months ago (2013-09-10 00:47:37 UTC) #1

Yoyo Zhou

LGTM https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_event_router.cc File chrome/browser/extensions/api/tabs/tabs_event_router.cc (right): https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_event_router.cc#newcode29 chrome/browser/extensions/api/tabs/tabs_event_router.cc:29: using base::ListValue; also could using FundamentalValue https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_event_router.h File ...

7 years, 3 months ago (2013-09-10 23:03:09 UTC) #3

Devlin

https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_event_router.cc File chrome/browser/extensions/api/tabs/tabs_event_router.cc (right): https://codereview.chromium.org/23724026/diff/3001/chrome/browser/extensions/api/tabs/tabs_event_router.cc#newcode29 chrome/browser/extensions/api/tabs/tabs_event_router.cc:29: using base::ListValue; On 2013/09/10 23:03:09, Yoyo Zhou wrote: > ...

7 years, 3 months ago (2013-09-11 00:49:50 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/23724026/14001

7 years, 3 months ago (2013-09-11 15:03:34 UTC) #5

commit-bot: I haz the power

Retried try job too often on win_rel for step(s) chrome_frame_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=196203

7 years, 3 months ago (2013-09-11 20:59:07 UTC) #6

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/23724026/14001

7 years, 3 months ago (2013-09-11 22:29:50 UTC) #7

Message was sent while issue was closed.

Change committed as 222660

Expand Messages | Collapse Messages