From bad77bf7b76ecaa884dd1cba56eeefd94e219c30 Mon Sep 17 00:00:00 2001 From: "aa@chromium.org" Date: Sat, 21 Aug 2010 21:21:22 +0000 Subject: Remove attempt to be smart about where to open navigations targetting app tabs. I futzed with this a bit to try and integrate installed apps extents, but it is complex. Also, I looked over the original bug where this code was added: crbug.com/29281. The issue there was that when you click on a bookmark (presumably in the bookmark bar) the pinned tab is unexpectedly overwritten. I can see that complaint, but there is also the use case of clicking a bookmark and intending it to overwrite the pinned tab. In summary, I kinda feel like the expectations about how navigation should work are too ingrained and we shouldn't be meddling. BUG=52680 Review URL: http://codereview.chromium.org/3161037 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@57002 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/extensions/extension_dom_ui.cc | 16 +++++++++------- chrome/browser/extensions/extension_dom_ui.h | 6 +++++- 2 files changed, 14 insertions(+), 8 deletions(-) (limited to 'chrome/browser/extensions') diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc index 598e43b..a91933f 100644 --- a/chrome/browser/extensions/extension_dom_ui.cc +++ b/chrome/browser/extensions/extension_dom_ui.cc @@ -22,6 +22,7 @@ #include "chrome/common/bindings_policy.h" #include "chrome/common/page_transition_types.h" #include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/extension_resource.h" #include "chrome/common/url_constants.h" #include "gfx/codec/png_codec.h" @@ -143,13 +144,14 @@ void ExtensionDOMUI::ResetExtensionFunctionDispatcher( } void ExtensionDOMUI::ResetExtensionBookmarkManagerEventRouter() { - extension_bookmark_manager_event_router_.reset( - new ExtensionBookmarkManagerEventRouter(GetProfile(), tab_contents())); - // We set the link transition type to AUTO_BOOKMARK for the bookmark manager. - // This doesn't really belong here, but neither does this function. - // ExtensionDOMUI could potentially be used for extensions besides the - // bookmark manager, but currently it does not. - link_transition_type_ = PageTransition::AUTO_BOOKMARK; + // Hack: A few things we specialize just for the bookmark manager. + if (extension_function_dispatcher_->extension_id() == + extension_misc::kBookmarkManagerId) { + extension_bookmark_manager_event_router_.reset( + new ExtensionBookmarkManagerEventRouter(GetProfile(), tab_contents())); + + link_transition_type_ = PageTransition::AUTO_BOOKMARK; + } } void ExtensionDOMUI::RenderViewCreated(RenderViewHost* render_view_host) { diff --git a/chrome/browser/extensions/extension_dom_ui.h b/chrome/browser/extensions/extension_dom_ui.h index d5f146d..56f1291 100644 --- a/chrome/browser/extensions/extension_dom_ui.h +++ b/chrome/browser/extensions/extension_dom_ui.h @@ -23,7 +23,9 @@ class TabContents; struct ViewHostMsg_DomMessage_Params; // This class implements DOMUI for extensions and allows extensions to put UI in -// the main tab contents area. +// the main tab contents area. For example, each extension can specify an +// "options_page", and that page is displayed in the tab contents area and is +// hosted by this class. class ExtensionDOMUI : public DOMUI, public ExtensionFunctionDispatcher::Delegate { @@ -92,6 +94,8 @@ class ExtensionDOMUI scoped_ptr extension_function_dispatcher_; + // TODO(aa): This seems out of place. Why is it not with the event routers for + // the other extension APIs? scoped_ptr extension_bookmark_manager_event_router_; }; -- cgit v1.1