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

Issue 159202: add mole info to toolstrip definition and enable moles (Closed)

Created:
11 years, 5 months ago by Erik does not do reviews
Modified:
9 years, 7 months ago
Reviewers:
Matt Perry
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

add mole info to toolstrip definition and enable moles Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21621

Patch Set 1 #

Patch Set 2 : ready for review #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+207 lines, -86 lines) Patch
M chrome/browser/extensions/extension_host.h View 1 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_host.cc View 1 6 chunks +39 lines, -14 lines 1 comment Download
M chrome/browser/extensions/extension_shelf_model.h View 1 4 chunks +12 lines, -4 lines 3 comments Download
M chrome/browser/extensions/extension_shelf_model.cc View 7 chunks +30 lines, -22 lines 0 comments Download
M chrome/browser/extensions/extensions_service_unittest.cc View 1 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/views/browser_bubble.h View 2 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/views/browser_bubble.cc View 1 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/views/extensions/extension_shelf.cc View 1 10 chunks +41 lines, -23 lines 2 comments Download
M chrome/browser/views/extensions/extension_view.h View 1 3 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/views/extensions/extension_view.cc View 1 2 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/views/frame/browser_view.cc View 1 chunk +4 lines, -5 lines 0 comments Download
M chrome/common/extensions/extension.h View 3 chunks +12 lines, -3 lines 0 comments Download
M chrome/common/extensions/extension.cc View 1 1 chunk +34 lines, -2 lines 2 comments Download
M chrome/common/extensions/extension_constants.h View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/extensions/extension_constants.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/notification_type.h View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Erik does not do reviews
11 years, 5 months ago (2009-07-24 22:05:48 UTC) #1
Matt Perry
LG overall. Can you add a sample extension with moles to the test/data dir (or ...
11 years, 5 months ago (2009-07-24 23:11:11 UTC) #2
Erik does not do reviews
I did add a mole to the loading tests, but didn't add the verification of ...
11 years, 5 months ago (2009-07-26 00:34:45 UTC) #3
Matt Perry
http://codereview.chromium.org/159202/diff/2001/20 File chrome/browser/extensions/extension_shelf_model.h (right): http://codereview.chromium.org/159202/diff/2001/20#newcode86 Line 86: int IndexOfToolstrip(ExtensionHost* toolstrip); On 2009/07/26 00:34:46, Erik Kay ...
11 years, 5 months ago (2009-07-27 17:41:50 UTC) #4
Erik does not do reviews
11 years, 5 months ago (2009-07-27 18:35:53 UTC) #5
On Mon, Jul 27, 2009 at 10:41 AM, <mpcomplete@chromium.org> wrote:

>
> http://codereview.chromium.org/159202/diff/2001/20
> File chrome/browser/extensions/extension_shelf_model.h (right):
>
> http://codereview.chromium.org/159202/diff/2001/20#newcode86
> Line 86: int IndexOfToolstrip(ExtensionHost* toolstrip);
> On 2009/07/26 00:34:46, Erik Kay wrote:
>
>> On 2009/07/24 23:11:11, Matt Perry wrote:
>> > IndexOfExtension?
>>
>
>  No.  An extension can have multiple toolstrips.
>>
>
> Then what good is this method?
>
> Actually, after a quick scan through the .cc files, it seems you only
> ever call this as IndexOfToolstrip(toolstrip->host).  You probably want
> it to take a Toolstrip* instead.


The use for this comes when we're outside of the shelf and just have an
ExtensionHost.

I'm working on a toolstrip API where the toolstrip can expand as a mole or
collapse back into the shelf.  In order to do this, we need to be able to
get from ExtensionHost to Toolstrip.

Perhaps it will be better to change IndexOfToolstrip to take a Toolstrip* as
you suggest and to add a ToolstripForHost for the other use case.  I'll look
at that as part of my next change.



> http://codereview.chromium.org/159202
>

Powered by Google App Engine
This is Rietveld 408576698