Added loading of nested BHO.

Added loading of nested BHO.

BUG=none
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71377

[Vitaly Buka (NO REVIEWS), 2011-01-12 23:35:46]

please review this

[Jói, 2011-01-13 01:22:11]

LGTM

Very nice. One nit and one thing to potentially change - I'm not sure but think
about it.

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.cc (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.cc:189: for (size_t i = 0; i <
nested_bho_.size(); ++i) {
I think this should go below the duplicate detection logic in the next block,
what do you think? My concern is that the main BHO may have uninitialized but
not ended in FinalRelease yet. It might not be an issue though.

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.h (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.h:471: // CEEE instead of.
instead of -> instead

[Vitaly Buka corp, 2011-01-13 01:46:17]

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.cc (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.cc:189: for (size_t i = 0; i <
nested_bho_.size(); ++i) {
In common case we don't know how nested BHO handles duplicates.

But seems duplicates logic is not correct.
According to this duplicates are possible and we should release old site.
http://msdn.microsoft.com/en-us/library/aa768221(v=vs.85).aspx
we need 

On 2011/01/13 01:22:11, Jói wrote:
> I think this should go below the duplicate detection logic in the next block,
> what do you think? My concern is that the main BHO may have uninitialized but
> not ended in FinalRelease yet. It might not be an issue though.

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.h (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.h:471: // CEEE instead of.
On 2011/01/13 01:22:11, Jói wrote:
> instead of -> instead

Done.

[Jói, 2011-01-13 19:18:03]

LGTM

Sorry for the delay, had a bunch of meetings today.

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.cc (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.cc:189: for (size_t i = 0; i <
nested_bho_.size(); ++i) {
On 2011/01/13 01:46:17, Vitaly Buka corp wrote:
> In common case we don't know how nested BHO handles duplicates.

OK, leave this as is then.

> But seems duplicates logic is not correct.
> According to this duplicates are possible and we should release old site.
> http://msdn.microsoft.com/en-us/library/aa768221%28v=vs.85%29.aspx

Hmm, I'm not sure. Take it up with Siggi, he wrote the code initially and might
know some more. Doesn't need to affect the current code review though.

[Sigurður Ásgeirsson, 2011-01-13 19:34:14]

lgtm wit a couple of nits.

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.cc (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.cc:189: for (size_t i = 0; i <
nested_bho_.size(); ++i) {
On 2011/01/13 01:46:17, Vitaly Buka corp wrote:
> In common case we don't know how nested BHO handles duplicates.
> 
> But seems duplicates logic is not correct.
> According to this duplicates are possible and we should release old site.
> http://msdn.microsoft.com/en-us/library/aa768221%28v=vs.85%29.aspx
> we need 

Interesting, you're quite right. I think I aped the logic from the classic
toolbar, feel free to fix this if you like.

http://codereview.chromium.org/6106004/diff/11001/ceee/ie/plugin/bho/browser_...
ceee/ie/plugin/bho/browser_helper_object.cc:126:
LoadString(_pModule->m_hInstResource, IDS_CEEE_NESTED_BHO_LIST,
we like to ::-prefix Win32 api calls.

http://codereview.chromium.org/6106004/diff/11001/ceee/ie/plugin/bho/browser_...
ceee/ie/plugin/bho/browser_helper_object.cc:136: if
(SUCCEEDED(CLSIDFromString(guids[i].c_str(), &clsid)) &&
Log on error.

[Vitaly Buka corp, 2011-01-13 21:06:45]

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
File ceee/ie/plugin/bho/browser_helper_object.cc (right):

http://codereview.chromium.org/6106004/diff/1/ceee/ie/plugin/bho/browser_help...
ceee/ie/plugin/bho/browser_helper_object.cc:189: for (size_t i = 0; i <
nested_bho_.size(); ++i) {
I guess this is not possible with IE, but classic one tries to handles this
correctly. However taking in account complexity of initialization and teardown I
have doubts that it will work :-).

Maybe I'll do this in special CL.

On 2011/01/13 19:34:15, Ruðrugis wrote:
> On 2011/01/13 01:46:17, Vitaly Buka corp wrote:
> > In common case we don't know how nested BHO handles duplicates.
> > 
> > But seems duplicates logic is not correct.
> > According to this duplicates are possible and we should release old site.
> > http://msdn.microsoft.com/en-us/library/aa768221%2528v=vs.85%2529.aspx
> > we need 
> 
> Interesting, you're quite right. I think I aped the logic from the classic
> toolbar, feel free to fix this if you like.

http://codereview.chromium.org/6106004/diff/11001/ceee/ie/plugin/bho/browser_...
ceee/ie/plugin/bho/browser_helper_object.cc:126:
LoadString(_pModule->m_hInstResource, IDS_CEEE_NESTED_BHO_LIST,
On 2011/01/13 19:34:15, Ruðrugis wrote:
> we like to ::-prefix Win32 api calls.

Done.

http://codereview.chromium.org/6106004/diff/11001/ceee/ie/plugin/bho/browser_...
ceee/ie/plugin/bho/browser_helper_object.cc:136: if
(SUCCEEDED(CLSIDFromString(guids[i].c_str(), &clsid)) &&
On 2011/01/13 19:34:15, Ruðrugis wrote:
> Log on error.

Done.