diff options
Diffstat (limited to 'chrome')
14 files changed, 195 insertions, 83 deletions
diff --git a/chrome/browser/extensions/extension_browser_event_router.cc b/chrome/browser/extensions/extension_browser_event_router.cc index 52d26da..1932552 100644 --- a/chrome/browser/extensions/extension_browser_event_router.cc +++ b/chrome/browser/extensions/extension_browser_event_router.cc @@ -22,61 +22,39 @@ namespace tab_keys = extension_tabs_module_constants; namespace page_action_keys = extension_page_actions_module_constants; ExtensionBrowserEventRouter::TabEntry::TabEntry() - : state_(ExtensionTabUtil::TAB_COMPLETE), - pending_navigate_(false), + : complete_waiting_on_load_(false), url_() { } -ExtensionBrowserEventRouter::TabEntry::TabEntry(const TabContents* contents) - : state_(ExtensionTabUtil::TAB_COMPLETE), - pending_navigate_(false), - url_(contents->GetURL()) { - UpdateLoadState(contents); -} - DictionaryValue* ExtensionBrowserEventRouter::TabEntry::UpdateLoadState( const TabContents* contents) { - ExtensionTabUtil::TabStatus old_state = state_; - state_ = ExtensionTabUtil::GetTabStatus(contents); - - if (old_state == state_) - return false; - - if (state_ == ExtensionTabUtil::TAB_LOADING) { - // Do not send "loading" state changed now. Wait for navigate so the new - // url is available. - pending_navigate_ = true; + // The tab may go in & out of loading (for instance if iframes navigate). + // We only want to respond to the first change from loading to !loading after + // the NAV_ENTRY_COMMITTED was fired. + if (!complete_waiting_on_load_ || contents->is_loading()) return NULL; - } else if (state_ == ExtensionTabUtil::TAB_COMPLETE) { - // Send "complete" state change. - DictionaryValue* changed_properties = new DictionaryValue(); - changed_properties->SetString(tab_keys::kStatusKey, - tab_keys::kStatusValueComplete); - return changed_properties; - - } else { - NOTREACHED(); - return NULL; - } + // Send "complete" state change. + complete_waiting_on_load_ = false; + DictionaryValue* changed_properties = new DictionaryValue(); + changed_properties->SetString(tab_keys::kStatusKey, + tab_keys::kStatusValueComplete); + return changed_properties; } DictionaryValue* ExtensionBrowserEventRouter::TabEntry::DidNavigate( const TabContents* contents) { - if (!pending_navigate_) - return NULL; - + // Send "loading" state change. + complete_waiting_on_load_ = true; DictionaryValue* changed_properties = new DictionaryValue(); changed_properties->SetString(tab_keys::kStatusKey, tab_keys::kStatusValueLoading); - GURL new_url = contents->GetURL(); - if (new_url != url_) { - url_ = new_url; + if (contents->GetURL() != url_) { + url_ = contents->GetURL(); changed_properties->SetString(tab_keys::kUrlKey, url_.spec()); } - pending_navigate_ = false; return changed_properties; } @@ -133,7 +111,7 @@ void ExtensionBrowserEventRouter::Init() { for (int i = 0; i < browser->tabstrip_model()->count(); ++i) { TabContents* contents = browser->tabstrip_model()->GetTabContentsAt(i); int tab_id = ExtensionTabUtil::GetTabId(contents); - tab_entries_[tab_id] = TabEntry(contents); + tab_entries_[tab_id] = TabEntry(); } } } @@ -225,7 +203,7 @@ void ExtensionBrowserEventRouter::TabInsertedAt(TabContents* contents, // If tab is new, send created event. int tab_id = ExtensionTabUtil::GetTabId(contents); if (tab_entries_.find(tab_id) == tab_entries_.end()) { - tab_entries_[tab_id] = TabEntry(contents); + tab_entries_[tab_id] = TabEntry(); TabCreatedAt(contents, index, foreground); return; diff --git a/chrome/browser/extensions/extension_browser_event_router.h b/chrome/browser/extensions/extension_browser_event_router.h index 13c8ead..f9b2367 100644 --- a/chrome/browser/extensions/extension_browser_event_router.h +++ b/chrome/browser/extensions/extension_browser_event_router.h @@ -115,13 +115,6 @@ class ExtensionBrowserEventRouter : public TabStripModelObserver, // std::map<> by value. TabEntry(); - // Create a new tab entry whose initial state is derived from the given - // tab contents. - explicit TabEntry(const TabContents* contents); - - // Returns the current state of the tab. - ExtensionTabUtil::TabStatus state() const { return state_; } - // Update the load state of the tab based on its TabContents. Returns true // if the state changed, false otherwise. Whether the state has changed or // not is used to determine if events needs to be sent to extensions during @@ -136,13 +129,11 @@ class ExtensionBrowserEventRouter : public TabStripModelObserver, DictionaryValue* DidNavigate(const TabContents* contents); private: - // Tab state used for last notification to extensions. - ExtensionTabUtil::TabStatus state_; - - // Remember that the LOADING state has been captured, but not yet reported - // because it is waiting on the navigation event to know what the - // destination url is. - bool pending_navigate_; + // Whether we are waiting to fire the 'complete' status change. This will + // occur the first time the TabContents stops loading after the + // NAV_ENTRY_COMMITTED was fired. The tab may go back into and out of the + // loading state subsequently, but we will ignore those changes. + bool complete_waiting_on_load_; GURL url_; }; diff --git a/chrome/browser/extensions/extension_tabs_apitest.cc b/chrome/browser/extensions/extension_tabs_apitest.cc index 7da6dad..d271af0 100644 --- a/chrome/browser/extensions/extension_tabs_apitest.cc +++ b/chrome/browser/extensions/extension_tabs_apitest.cc @@ -37,3 +37,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, CaptureVisibleTab) { ASSERT_TRUE(RunExtensionTest("tabs/capture_visible_tab")) << message_; } + +IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TabsOnUpdated) { + ASSERT_TRUE(StartHTTPServer()); + + ASSERT_TRUE(RunExtensionTest("tabs/on_updated")) << message_; +} diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc index 6912e25..771ad23 100644 --- a/chrome/browser/extensions/extension_tabs_module.cc +++ b/chrome/browser/extensions/extension_tabs_module.cc @@ -78,23 +78,8 @@ int ExtensionTabUtil::GetTabId(const TabContents* tab_contents) { return tab_contents->controller().session_id().id(); } -ExtensionTabUtil::TabStatus ExtensionTabUtil::GetTabStatus( - const TabContents* tab_contents) { - return tab_contents->is_loading() ? TAB_LOADING : TAB_COMPLETE; -} - -std::string ExtensionTabUtil::GetTabStatusText(TabStatus status) { - std::string text; - switch (status) { - case TAB_LOADING: - text = keys::kStatusValueLoading; - break; - case TAB_COMPLETE: - text = keys::kStatusValueComplete; - break; - } - - return text; +std::string ExtensionTabUtil::GetTabStatusText(bool is_loading) { + return is_loading ? keys::kStatusValueLoading : keys::kStatusValueComplete; } int ExtensionTabUtil::GetWindowIdOfTab(const TabContents* tab_contents) { @@ -130,22 +115,20 @@ ListValue* ExtensionTabUtil::CreateTabList(const Browser* browser) { DictionaryValue* ExtensionTabUtil::CreateTabValue( const TabContents* contents, TabStripModel* tab_strip, int tab_index) { - TabStatus status = GetTabStatus(contents); - DictionaryValue* result = new DictionaryValue(); result->SetInteger(keys::kIdKey, ExtensionTabUtil::GetTabId(contents)); result->SetInteger(keys::kIndexKey, tab_index); result->SetInteger(keys::kWindowIdKey, ExtensionTabUtil::GetWindowIdOfTab(contents)); result->SetString(keys::kUrlKey, contents->GetURL().spec()); - result->SetString(keys::kStatusKey, GetTabStatusText(status)); + result->SetString(keys::kStatusKey, GetTabStatusText(contents->is_loading())); result->SetBoolean(keys::kSelectedKey, tab_strip && tab_index == tab_strip->selected_index()); result->SetString(keys::kTitleKey, UTF16ToWide(contents->GetTitle())); result->SetBoolean(keys::kIncognitoKey, contents->profile()->IsOffTheRecord()); - if (status != TAB_LOADING) { + if (!contents->is_loading()) { NavigationEntry* entry = contents->controller().GetActiveEntry(); if (entry) { if (entry->favicon().is_valid()) diff --git a/chrome/browser/extensions/extension_tabs_module.h b/chrome/browser/extensions/extension_tabs_module.h index 8e82981..432ee22 100644 --- a/chrome/browser/extensions/extension_tabs_module.h +++ b/chrome/browser/extensions/extension_tabs_module.h @@ -21,17 +21,9 @@ class TabStripModel; class ExtensionTabUtil { public: - // Possible tab states. These states are used to calculate the "status" - // property of the Tab object that is used in the extension tab API. - enum TabStatus { - TAB_LOADING, // Waiting for the DOM to load. - TAB_COMPLETE // Tab loading and rendering is complete. - }; - static int GetWindowId(const Browser* browser); static int GetTabId(const TabContents* tab_contents); - static TabStatus GetTabStatus(const TabContents* tab_contents); - static std::string GetTabStatusText(TabStatus status); + static std::string GetTabStatusText(bool is_loading); static int GetWindowIdOfTab(const TabContents* tab_contents); static ListValue* CreateTabList(const Browser* browser); static DictionaryValue* CreateTabValue(const TabContents* tab_contents); diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/browserThenRendererInitiated/a.html b/chrome/test/data/extensions/api_test/tabs/on_updated/browserThenRendererInitiated/a.html new file mode 100644 index 0000000..e2fa12e --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/browserThenRendererInitiated/a.html @@ -0,0 +1,7 @@ +<h1>A (browser initiated)</h1> + +<script> +window.onload = function() { + location.href = "b.html"; +} +</script>
\ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/browserThenRendererInitiated/b.html b/chrome/test/data/extensions/api_test/tabs/on_updated/browserThenRendererInitiated/b.html new file mode 100644 index 0000000..64cf281 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/browserThenRendererInitiated/b.html @@ -0,0 +1 @@ +<h1>B (renderer initiated)</h1> diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/a.html b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/a.html new file mode 100644 index 0000000..3429290 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/a.html @@ -0,0 +1,3 @@ +<h1>iframes Navigated</h1> + +<iframe src="iframe1.html"></iframe>
\ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe1.html b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe1.html new file mode 100644 index 0000000..6e87ff4 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe1.html @@ -0,0 +1,7 @@ +<h1>iframe1</h1> +<script> +window.onload = function() { + // Navigate during the onload so that 'complete' status won't fire. + location.href = "iframe2.html"; +} +</script>
\ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe2.html b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe2.html new file mode 100644 index 0000000..f1fb9a9 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe2.html @@ -0,0 +1,10 @@ +<h1>iframe2</h1> + +<script> +window.onload = function() { + // Navigate after onload so that 'complete' status will fire. + setTimeout(function() { + location.href = "iframe3.html"; + }, 0); +} +</script>
\ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe3.html b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe3.html new file mode 100644 index 0000000..6904d24 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/iframeNavigated/iframe3.html @@ -0,0 +1 @@ +<h1>iframe3</h1>
\ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/internalAnchorNavigated/a.html b/chrome/test/data/extensions/api_test/tabs/on_updated/internalAnchorNavigated/a.html new file mode 100644 index 0000000..ef2b1ef --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/internalAnchorNavigated/a.html @@ -0,0 +1,14 @@ +<h1>Internal Anchor</h1> + +<a href="#b">b</a> +<div style="height: 1600px">---SPACE---</div> +<h2 id="b">B Content</h2> + +<script> +window.onload = function() { + // Navigate after the onload, so the 'complete' status will fire. + setTimeout(function() { + location.href = "#b"; + }, 0); +} +</script>
\ No newline at end of file diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json b/chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json new file mode 100644 index 0000000..0a5e419 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json @@ -0,0 +1,7 @@ +{ + "name": "tabs/on_updated", + "version": "0.1", + "description": "Tests the behavior of the tabs.onUpdated event with different cases of navigation", + "background_page": "test.html", + "permissions": ["tabs"] +} diff --git a/chrome/test/data/extensions/api_test/tabs/on_updated/test.html b/chrome/test/data/extensions/api_test/tabs/on_updated/test.html new file mode 100644 index 0000000..284c6f7 --- /dev/null +++ b/chrome/test/data/extensions/api_test/tabs/on_updated/test.html @@ -0,0 +1,112 @@ +<script> +// Description + +var expectedEventData; +var capturedEventData; +var shouldIgnore; + +function expect(data, ignoreFunc) { + expectedEventData = data; + capturedEventData = []; + shouldIgnore = ignoreFunc; +} + +function checkExpectations() { + if (capturedEventData.length < expectedEventData.length) { + return; + } + chrome.test.assertEq(JSON.stringify(expectedEventData), + JSON.stringify(capturedEventData)); + chrome.test.succeed(); +} + +var getURL = chrome.extension.getURL; + +chrome.tabs.onUpdated.addListener(function(tabId, info, tab) { + console.log('---onUpdated: ' + info.status + ', ' + info.url); + if (shouldIgnore && shouldIgnore(info)) { + return; + } + capturedEventData.push(info); + checkExpectations(); +}); + +chrome.test.runTests([ + function browserThenRendererInitiated() { + // Note that a.html will set it's location.href to b.html, creating a + // renderer-initiated navigation. + expect([ + { status: 'loading', url: getURL('browserThenRendererInitiated/a.html') }, + { status: 'complete' }, + { status: 'loading', url: getURL('browserThenRendererInitiated/b.html') }, + { status: 'complete' }, + ]); + + chrome.tabs.create({ url: getURL('browserThenRendererInitiated/a.html') }); + }, + + function newTab() { + // Test for crbug.com/27208. + expect([ + { status: 'loading', url: 'chrome://newtab/' }, + { status: 'complete' } + ]); + + chrome.tabs.create({ url: 'chrome://newtab/' }); + }, + + /* + // TODO(rafaelw) -- This is disabled because this test is flakey. + function updateDuringCreateCallback() { + // Test for crbug.com/27204. + // We have to ignore anything that comes before the about:blank loading + // status. + var ignore = true; + expect([ + { status: 'loading', url: 'about:blank' }, + { status: 'complete' } + ], function(info) { + if (info.status === 'loading' && info.url === 'about:blank') { + ignore = false; + } + return ignore; + }); + + chrome.tabs.create({ url: 'chrome://newtab/' }, function(tab) { + chrome.tabs.update(tab.id, { url: 'about:blank' }); + }); + }, */ + + function iframeNavigated() { + // The sequence of events goes like this: + // -a.html starts loading + // -while a.html is loading, iframe1.html (in it's onload) navigates to + // iframe2.html. This causes the page to continue to be in the loading state + // so the 'complete' status doesn't fire. + // -iframe2.html does a setTimeout to navigate itself to iframe3.html. This + // allows the page to stop loading and the 'complete' status to fire, but + // when the timeout fires, the pages goes back into the loading state + // which causes the new status: 'loading' event to fire without having + // changed the url. + expect([ + { status: 'loading', url: getURL('iframeNavigated/a.html') }, + { status: 'complete' }, + { status: 'loading' }, + { status: 'complete' }, + ]); + + chrome.tabs.create({ url: getURL('iframeNavigated/a.html') }); + }, + + function internalAnchorNavigated() { + expect([ + { status: 'loading', url: getURL('internalAnchorNavigated/a.html') }, + { status: 'complete' }, + { status: 'loading', url: getURL('internalAnchorNavigated/a.html#b') }, + { status: 'complete' }, + ]); + + chrome.tabs.create({ url: getURL('internalAnchorNavigated/a.html') }); + } +]); +</script>
\ No newline at end of file |