Issue 11412247: Removed dead code from chrome/browser/extensions/api/declarative

Issue 11412247: Removed dead code from chrome/browser/extensions/api/declarative (Closed)

Created:
8 years ago by Sergiy Byelozyorov (Google)

Modified:
8 years ago

Reviewers:
Matt Perry, battre

CC:
chromium-reviews, Aaron Boodman, chromium-apps-reviews_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Removed dead code from chrome/browser/extensions/api/declarative BUG=161992 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170319

Patch Set 1 #

Total comments: 2

Created: 8 years ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+0 lines, -12 lines)			Patch
	M	chrome/browser/extensions/api/declarative/rules_registry_with_cache.h	View		1 chunk	+0 lines, -8 lines	0 comments	Download
	M	chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc	View		1 chunk	+0 lines, -4 lines	2 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergiyb@google.com/11412247/1

8 years ago (2012-11-29 22:01:54 UTC) #3

battre

https://chromiumcodereview.appspot.com/11412247/diff/1/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc File chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc (left): https://chromiumcodereview.appspot.com/11412247/diff/1/chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc#oldcode30 chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc:30: ready_callbacks_.clear(); I think this is now broken. You cannot ...

8 years ago (2012-12-01 02:27:54 UTC) #5

Matt Perry

8 years ago (2012-12-03 17:56:19 UTC) #6

Message was sent while issue was closed.

https://chromiumcodereview.appspot.com/11412247/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc
(left):

https://chromiumcodereview.appspot.com/11412247/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/declarative/rules_registry_with_cache.cc:30:
ready_callbacks_.clear();
On 2012/12/01 02:27:54, battre wrote:
> I think this is now broken. You cannot add read_callbacks_ anymore but we try
to
> trigger them here.
> 
> I don't have a working code index right now. We should check whether OnReady()
> gets called by anybody. If this is the case, we should consider whether it is
> necessary or whether we are dealing with a bug here. I am kind of expecting
that
> but haven't had the time to research, yet.

I should've looked at this more closely. There's a caller of AddReadyCallback
here:
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/extensions/...

It's behind an #ifdef, so it doesn't actually compile. But I believe we will
eventually want to flip the #ifdef to ON.

We should revert this change.

Expand Messages | Collapse Messages