diff options
10 files changed, 164 insertions, 35 deletions
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc index d470801..e225583 100644 --- a/content/browser/browser_plugin/browser_plugin_guest.cc +++ b/content/browser/browser_plugin/browser_plugin_guest.cc @@ -383,14 +383,17 @@ void BrowserPluginGuest::DidCommitProvisionalLoadForFrame( PageTransition transition_type, RenderViewHost* render_view_host) { // Inform its embedder of the updated URL. - if (is_main_frame) { - SendMessageToEmbedder( - new BrowserPluginMsg_DidNavigate( - instance_id(), - url, - render_view_host->GetProcess()->GetID())); - RecordAction(UserMetricsAction("BrowserPlugin.Guest.DidNavigate")); - } + BrowserPluginMsg_DidNavigate_Params params; + params.url = url; + params.is_top_level = is_main_frame; + params.process_id = render_view_host->GetProcess()->GetID(); + params.current_entry_index = + web_contents()->GetController().GetCurrentEntryIndex(); + params.entry_count = + web_contents()->GetController().GetEntryCount(); + SendMessageToEmbedder( + new BrowserPluginMsg_DidNavigate(instance_id(), params)); + RecordAction(UserMetricsAction("BrowserPlugin.Guest.DidNavigate")); } void BrowserPluginGuest::RenderViewGone(base::TerminationStatus status) { diff --git a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc index 703a4f9..4f19031 100644 --- a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc +++ b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc @@ -550,6 +550,19 @@ IN_PROC_BROWSER_TEST_F(BrowserPluginHostTest, Renavigate) { string16 actual_title = title_watcher.WaitAndGetTitle(); EXPECT_EQ(expected_title, actual_title); + + base::Value* value = + rvh->ExecuteJavascriptAndGetValue(string16(), + ASCIIToUTF16("CanGoBack()")); + bool result = false; + ASSERT_TRUE(value->GetAsBoolean(&result)); + EXPECT_TRUE(result); + + value = rvh->ExecuteJavascriptAndGetValue(string16(), + ASCIIToUTF16("CanGoForward()")); + result = false; + ASSERT_TRUE(value->GetAsBoolean(&result)); + EXPECT_TRUE(result); } // Go forward and verify that we're back at P3. @@ -562,6 +575,13 @@ IN_PROC_BROWSER_TEST_F(BrowserPluginHostTest, Renavigate) { string16 actual_title = title_watcher.WaitAndGetTitle(); EXPECT_EQ(expected_title, actual_title); + + base::Value* value = + rvh->ExecuteJavascriptAndGetValue(string16(), + ASCIIToUTF16("CanGoForward()")); + bool result = true; + ASSERT_TRUE(value->GetAsBoolean(&result)); + EXPECT_FALSE(result); } // Go back two entries and verify that we're back at P1. @@ -574,6 +594,13 @@ IN_PROC_BROWSER_TEST_F(BrowserPluginHostTest, Renavigate) { string16 actual_title = title_watcher.WaitAndGetTitle(); EXPECT_EQ(expected_title, actual_title); + + base::Value* value = + rvh->ExecuteJavascriptAndGetValue(string16(), + ASCIIToUTF16("CanGoBack()")); + bool result = true; + ASSERT_TRUE(value->GetAsBoolean(&result)); + EXPECT_FALSE(result); } } diff --git a/content/common/browser_plugin_messages.h b/content/common/browser_plugin_messages.h index 6bc5154..67520a8 100644 --- a/content/common/browser_plugin_messages.h +++ b/content/common/browser_plugin_messages.h @@ -160,12 +160,26 @@ IPC_MESSAGE_CONTROL4(BrowserPluginMsg_LoadRedirect, GURL /* new_url */, bool /* is_top_level */) +IPC_STRUCT_BEGIN(BrowserPluginMsg_DidNavigate_Params) + // The current URL of the guest. + IPC_STRUCT_MEMBER(GURL, url) + // Indicates whether the navigation was on the top-level frame. + IPC_STRUCT_MEMBER(bool, is_top_level) + // Chrome's process ID for the guest. + IPC_STRUCT_MEMBER(int, process_id) + // The index of the current navigation entry after this navigation was + // committed. + IPC_STRUCT_MEMBER(int, current_entry_index) + // The number of navigation entries after this navigation was committed. + IPC_STRUCT_MEMBER(int, entry_count) +IPC_STRUCT_END() + // When the guest navigates, the browser process informs the embedder through -// the BrowserPluginMsg_DidNavigate message. -IPC_MESSAGE_CONTROL3(BrowserPluginMsg_DidNavigate, +// the BrowserPluginMsg_DidNavigate message along with information about the +// guest's current navigation state. +IPC_MESSAGE_CONTROL2(BrowserPluginMsg_DidNavigate, int /* instance_id */, - GURL /* url */, - int /* process_id */) + BrowserPluginMsg_DidNavigate_Params) // When the guest crashes, the browser process informs the embedder through this // message. diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc index adde725..294cec0 100644 --- a/content/renderer/browser_plugin/browser_plugin.cc +++ b/content/renderer/browser_plugin/browser_plugin.cc @@ -72,7 +72,9 @@ BrowserPlugin::BrowserPlugin( navigate_src_sent_(false), process_id_(-1), persist_storage_(false), - visible_(true) { + visible_(true), + current_nav_entry_index_(0), + nav_entry_count_(0) { BrowserPluginManager::Get()->AddBrowserPlugin(instance_id, this); bindings_.reset(new BrowserPluginBindings(this)); @@ -149,6 +151,15 @@ std::string BrowserPlugin::GetPartitionAttribute() const { return value; } +bool BrowserPlugin::CanGoBack() const { + return nav_entry_count_ > 1 && current_nav_entry_index_ > 0; +} + +bool BrowserPlugin::CanGoForward() const { + return current_nav_entry_index_ >= 0 && + current_nav_entry_index_ < (nav_entry_count_ - 1); +} + bool BrowserPlugin::SetPartitionAttribute(const std::string& partition_id, std::string& error_message) { if (navigate_src_sent_) { @@ -344,9 +355,13 @@ void BrowserPlugin::GuestCrashed() { } } -void BrowserPlugin::DidNavigate(const GURL& url, int process_id) { - src_ = url.spec(); - process_id_ = process_id; +void BrowserPlugin::DidNavigate( + const BrowserPluginMsg_DidNavigate_Params& params) { + src_ = params.url.spec(); + process_id_ = params.process_id; + current_nav_entry_index_ = params.current_entry_index; + nav_entry_count_ = params.entry_count; + if (!HasListeners(kNavigationEventName)) return; @@ -355,7 +370,13 @@ void BrowserPlugin::DidNavigate(const GURL& url, int process_id) { v8::Context::Scope context_scope( plugin.document().frame()->mainWorldScriptContext()); - v8::Local<v8::Value> param = v8::String::New(src_.data(), src_.size()); + // Construct the navigation event object. + v8::Local<v8::Object> event = v8::Object::New(); + event->Set(v8::String::New(kURL, sizeof(kURL) - 1), + v8::String::New(src_.data(), src_.size())); + event->Set(v8::String::New(kIsTopLevel, sizeof(kIsTopLevel) - 1), + v8::Boolean::New(params.is_top_level)); + v8::Local<v8::Value> val = event; // TODO(fsamuel): Copying the event listeners is insufficent because // new persistent handles are not created when the copy constructor is @@ -366,7 +387,7 @@ void BrowserPlugin::DidNavigate(const GURL& url, int process_id) { WebKit::WebFrame* frame = plugin.document().frame(); if (frame) { frame->callFunctionEvenIfScriptDisabled( - *it, v8::Object::New(), 1, ¶m); + *it, v8::Object::New(), 1, &val); } } } diff --git a/content/renderer/browser_plugin/browser_plugin.h b/content/renderer/browser_plugin/browser_plugin.h index 45ae96d..6c87869 100644 --- a/content/renderer/browser_plugin/browser_plugin.h +++ b/content/renderer/browser_plugin/browser_plugin.h @@ -17,6 +17,7 @@ #include "content/renderer/render_view_impl.h" struct BrowserPluginHostMsg_ResizeGuest_Params; +struct BrowserPluginMsg_DidNavigate_Params; struct BrowserPluginMsg_UpdateRect_Params; namespace content { @@ -40,6 +41,10 @@ class CONTENT_EXPORT BrowserPlugin : int process_id() const { return process_id_; } // The partition identifier string is stored as UTF-8. std::string GetPartitionAttribute() const; + // Query whether the guest can navigate back to the previous entry. + bool CanGoBack() const; + // Query whether the guest can navigation forward to the next entry. + bool CanGoForward() const; // This method can be successfully called only before the first navigation for // this instance of BrowserPlugin. If an error occurs, the |error_message| is // set appropriately to indicate the failure reason. @@ -53,7 +58,7 @@ class CONTENT_EXPORT BrowserPlugin : // Inform the BrowserPlugin that its guest has crashed. void GuestCrashed(); // Informs the BrowserPlugin that the guest has navigated to a new URL. - void DidNavigate(const GURL& url, int process_id); + void DidNavigate(const BrowserPluginMsg_DidNavigate_Params& params); // Inform the BrowserPlugin that the guest has started loading a new page. void LoadStart(const GURL& url, bool is_top_level); // Inform the BrowserPlugin that the guest has aborted loading a new page. @@ -200,6 +205,18 @@ class CONTENT_EXPORT BrowserPlugin : #if defined(OS_WIN) base::SharedMemory shared_memory_; #endif + // Important: Do not add more history state here. + // We strongly discourage storing additional history state (such as page IDs) + // in the embedder process, at the risk of having incorrect information that + // can lead to broken back/forward logic in apps. + // It's also important that this state does not get modified by any logic in + // the embedder process. It should only be updated in response to navigation + // events in the guest. No assumptions should be made about how the index + // will change after a navigation (e.g., for back, forward, or go), because + // the changes are not always obvious. For example, there is a maximum + // number of entries and earlier ones will automatically be pruned. + int current_nav_entry_index_; + int nav_entry_count_; DISALLOW_COPY_AND_ASSIGN(BrowserPlugin); }; diff --git a/content/renderer/browser_plugin/browser_plugin_bindings.cc b/content/renderer/browser_plugin/browser_plugin_bindings.cc index 56230a5..afb0556 100644 --- a/content/renderer/browser_plugin/browser_plugin_bindings.cc +++ b/content/renderer/browser_plugin/browser_plugin_bindings.cc @@ -39,6 +39,8 @@ namespace { const char kAddEventListener[] = "addEventListener"; const char kBackMethod[] = "back"; +const char kCanGoBack[] = "canGoBack"; +const char kCanGoForward[] = "canGoForward"; const char kForwardMethod[] = "forward"; const char kGetProcessId[] = "getProcessId"; const char kGoMethod[] = "go"; @@ -62,6 +64,14 @@ bool IdentifierIsBackMethod(NPIdentifier identifier) { return WebBindings::getStringIdentifier(kBackMethod) == identifier; } +bool IdentifierIsCanGoBack(NPIdentifier identifier) { + return WebBindings::getStringIdentifier(kCanGoBack) == identifier; +} + +bool IdentifierIsCanGoForward(NPIdentifier identifier) { + return WebBindings::getStringIdentifier(kCanGoForward) == identifier; +} + bool IdentifierIsForwardMethod(NPIdentifier identifier) { return WebBindings::getStringIdentifier(kForwardMethod) == identifier; } @@ -151,6 +161,12 @@ bool BrowserPluginBindingsHasMethod(NPObject* np_obj, NPIdentifier name) { if (IdentifierIsBackMethod(name)) return true; + if (IdentifierIsCanGoBack(name)) + return true; + + if (IdentifierIsCanGoForward(name)) + return true; + if (IdentifierIsForwardMethod(name)) return true; @@ -207,6 +223,18 @@ bool BrowserPluginBindingsInvoke(NPObject* np_obj, NPIdentifier name, return true; } + if (IdentifierIsCanGoBack(name) && !arg_count) { + result->type = NPVariantType_Bool; + result->value.boolValue = bindings->instance()->CanGoBack(); + return true; + } + + if (IdentifierIsCanGoForward(name) && !arg_count) { + result->type = NPVariantType_Bool; + result->value.boolValue = bindings->instance()->CanGoForward(); + return true; + } + if (IdentifierIsForwardMethod(name) && !arg_count) { bindings->instance()->Forward(); return true; diff --git a/content/renderer/browser_plugin/browser_plugin_browsertest.cc b/content/renderer/browser_plugin/browser_plugin_browsertest.cc index 3aa7527..b03bc31 100644 --- a/content/renderer/browser_plugin/browser_plugin_browsertest.cc +++ b/content/renderer/browser_plugin/browser_plugin_browsertest.cc @@ -312,8 +312,8 @@ TEST_F(BrowserPluginTest, RemovePlugin) { TEST_F(BrowserPluginTest, CustomEvents) { const char* kAddEventListener = "var url;" - "function nav(u) {" - " url = u;" + "function nav(e) {" + " url = e.url;" "}" "document.getElementById('browserplugin')." " addEventListener('navigation', nav);"; @@ -342,16 +342,25 @@ TEST_F(BrowserPluginTest, CustomEvents) { browser_plugin_manager()->GetBrowserPlugin(instance_id)); ASSERT_TRUE(browser_plugin); - browser_plugin->DidNavigate(GURL(kGoogleURL), 1337); - EXPECT_EQ(kGoogleURL, ExecuteScriptAndReturnString("url")); - EXPECT_EQ(1337, ExecuteScriptAndReturnInt(kGetProcessID)); - + { + BrowserPluginMsg_DidNavigate_Params navigate_params; + navigate_params.url = GURL(kGoogleURL); + navigate_params.process_id = 1337; + browser_plugin->DidNavigate(navigate_params); + EXPECT_EQ(kGoogleURL, ExecuteScriptAndReturnString("url")); + EXPECT_EQ(1337, ExecuteScriptAndReturnInt(kGetProcessID)); + } ExecuteJavaScript(kRemoveEventListener); - browser_plugin->DidNavigate(GURL(kGoogleNewsURL), 42); - // The URL variable should not change because we've removed the event - // listener. - EXPECT_EQ(kGoogleURL, ExecuteScriptAndReturnString("url")); - EXPECT_EQ(42, ExecuteScriptAndReturnInt(kGetProcessID)); + { + BrowserPluginMsg_DidNavigate_Params navigate_params; + navigate_params.url = GURL(kGoogleNewsURL); + navigate_params.process_id = 42; + browser_plugin->DidNavigate(navigate_params); + // The URL variable should not change because we've removed the event + // listener. + EXPECT_EQ(kGoogleURL, ExecuteScriptAndReturnString("url")); + EXPECT_EQ(42, ExecuteScriptAndReturnInt(kGetProcessID)); + } } TEST_F(BrowserPluginTest, StopMethod) { diff --git a/content/renderer/browser_plugin/browser_plugin_manager_impl.cc b/content/renderer/browser_plugin/browser_plugin_manager_impl.cc index d4027f3..a88b34f 100644 --- a/content/renderer/browser_plugin/browser_plugin_manager_impl.cc +++ b/content/renderer/browser_plugin/browser_plugin_manager_impl.cc @@ -64,12 +64,12 @@ void BrowserPluginManagerImpl::OnGuestCrashed(int instance_id) { plugin->GuestCrashed(); } -void BrowserPluginManagerImpl::OnDidNavigate(int instance_id, - const GURL& url, - int process_id) { +void BrowserPluginManagerImpl::OnDidNavigate( + int instance_id, + const BrowserPluginMsg_DidNavigate_Params& params) { BrowserPlugin* plugin = GetBrowserPlugin(instance_id); if (plugin) - plugin->DidNavigate(url, process_id); + plugin->DidNavigate(params); } void BrowserPluginManagerImpl::OnAdvanceFocus(int instance_id, bool reverse) { diff --git a/content/renderer/browser_plugin/browser_plugin_manager_impl.h b/content/renderer/browser_plugin/browser_plugin_manager_impl.h index ccfe884..9a53f78 100644 --- a/content/renderer/browser_plugin/browser_plugin_manager_impl.h +++ b/content/renderer/browser_plugin/browser_plugin_manager_impl.h @@ -8,6 +8,7 @@ #include "content/renderer/browser_plugin/browser_plugin_manager.h" #include "googleurl/src/gurl.h" +struct BrowserPluginMsg_DidNavigate_Params; struct BrowserPluginMsg_UpdateRect_Params; namespace content { @@ -33,7 +34,8 @@ class BrowserPluginManagerImpl : public BrowserPluginManager { int message_id, const BrowserPluginMsg_UpdateRect_Params& params); void OnGuestCrashed(int instance_id); - void OnDidNavigate(int instance_id, const GURL& url, int process_id); + void OnDidNavigate(int instance_id, + const BrowserPluginMsg_DidNavigate_Params& params); void OnAdvanceFocus(int instance_id, bool reverse); void OnShouldAcceptTouchEvents(int instance_id, bool accept); void OnLoadStart(int instance_id, diff --git a/content/test/data/browser_plugin_embedder.html b/content/test/data/browser_plugin_embedder.html index 45b374e..e192865 100644 --- a/content/test/data/browser_plugin_embedder.html +++ b/content/test/data/browser_plugin_embedder.html @@ -27,6 +27,14 @@ function Back() { var plugin = document.getElementById('plugin'); plugin.back(); } +function CanGoBack() { + var plugin = document.getElementById('plugin'); + return plugin.canGoBack(); +} +function CanGoForward() { + var plugin = document.getElementById('plugin'); + return plugin.canGoForward(); +} function Forward() { var plugin = document.getElementById('plugin'); plugin.forward(); |