Bug 1309900: Fix subframe history navigation logic.
Renamed CompareFrames to LoadDifferingEntries to make what it does more clear.
Also, use this to do the top level load for the forward / back cases as it is pretty much set up to do that already.
MozReview-Commit-ID: G8cO0jDZebR
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -1531,17 +1531,20 @@ nsSHistory::LoadNextPossibleEntry(int32_
return LoadEntry(aNewIndex + 1, aLoadType, aHistCmd);
}
return NS_ERROR_FAILURE;
}
NS_IMETHODIMP
nsSHistory::LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd)
{
- nsCOMPtr<nsIDocShell> docShell;
+ if (!mRootDocShell) {
+ return NS_ERROR_FAILURE;
+ }
+
// Keep note of requested history index in mRequestedIndex.
mRequestedIndex = aIndex;
nsCOMPtr<nsISHEntry> prevEntry;
GetEntryAtIndex(mIndex, false, getter_AddRefs(prevEntry));
nsCOMPtr<nsISHEntry> nextEntry;
GetEntryAtIndex(mRequestedIndex, false, getter_AddRefs(nextEntry));
@@ -1579,112 +1582,65 @@ nsSHistory::LoadEntry(int32_t aIndex, lo
if (!canNavigate) {
// If the listener asked us not to proceed with
// the operation, simply return.
mRequestedIndex = -1;
return NS_OK; // XXX Maybe I can return some other error code?
}
- int32_t pCount = 0;
- int32_t nCount = 0;
- nsCOMPtr<nsISHContainer> prevAsContainer(do_QueryInterface(prevEntry));
- nsCOMPtr<nsISHContainer> nextAsContainer(do_QueryInterface(nextEntry));
- if (prevAsContainer && nextAsContainer) {
- prevAsContainer->GetChildCount(&pCount);
- nextAsContainer->GetChildCount(&nCount);
- }
-
if (mRequestedIndex == mIndex) {
// Possibly a reload case
- docShell = mRootDocShell;
- } else {
- // Going back or forward.
- if (pCount > 0 && nCount > 0) {
- /* THis is a subframe navigation. Go find
- * the docshell in which load should happen
- */
- bool frameFound = false;
- nsresult rv = CompareFrames(prevEntry, nextEntry, mRootDocShell,
- aLoadType, &frameFound);
- if (!frameFound) {
- // We did not successfully find the subframe in which
- // the new url was to be loaded. Go further in the history.
- return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd);
- }
- return rv;
- } else {
- // Loading top level page.
- uint32_t prevID = 0;
- uint32_t nextID = 0;
- prevEntry->GetID(&prevID);
- nextEntry->GetID(&nextID);
- if (prevID == nextID) {
- // Try harder to find something new to load.
- // This may happen for example if some page removed iframes dynamically.
- return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd);
- }
- docShell = mRootDocShell;
- }
+ return InitiateLoad(nextEntry, mRootDocShell, aLoadType);
}
- if (!docShell) {
- // we did not successfully go to the proper index.
- // return error.
- mRequestedIndex = -1;
- return NS_ERROR_FAILURE;
+ // Going back or forward.
+ bool differenceFound = false;
+ nsresult rv = LoadDifferingEntries(prevEntry, nextEntry, mRootDocShell,
+ aLoadType, differenceFound);
+ if (!differenceFound) {
+ // We did not find any differences. Go further in the history.
+ return LoadNextPossibleEntry(aIndex, aLoadType, aHistCmd);
}
- // Start the load on the appropriate docshell
- return InitiateLoad(nextEntry, docShell, aLoadType);
+ return rv;
}
nsresult
-nsSHistory::CompareFrames(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
- nsIDocShell* aParent, long aLoadType,
- bool* aIsFrameFound)
+nsSHistory::LoadDifferingEntries(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
+ nsIDocShell* aParent, long aLoadType,
+ bool& aDifferenceFound)
{
if (!aPrevEntry || !aNextEntry || !aParent) {
return NS_ERROR_FAILURE;
}
- // We should be comparing only entries which were created for the
- // same docshell. This is here to just prevent anything strange happening.
- // This check could be possibly an assertion.
- uint64_t prevdID, nextdID;
- aPrevEntry->GetDocshellID(&prevdID);
- aNextEntry->GetDocshellID(&nextdID);
- NS_ENSURE_STATE(prevdID == nextdID);
-
nsresult result = NS_OK;
uint32_t prevID, nextID;
aPrevEntry->GetID(&prevID);
aNextEntry->GetID(&nextID);
// Check the IDs to verify if the pages are different.
if (prevID != nextID) {
- if (aIsFrameFound) {
- *aIsFrameFound = true;
- }
- // Set the Subframe flag of the entry to indicate that
- // it is subframe navigation
- aNextEntry->SetIsSubFrame(true);
- InitiateLoad(aNextEntry, aParent, aLoadType);
- return NS_OK;
+ aDifferenceFound = true;
+
+ // Set the Subframe flag if not navigating the root docshell.
+ aNextEntry->SetIsSubFrame(aParent != mRootDocShell);
+ return InitiateLoad(aNextEntry, aParent, aLoadType);
}
- // The root entries are the same, so compare any child frames
+ // The entries are the same, so compare any child frames
int32_t pcnt = 0;
int32_t ncnt = 0;
int32_t dsCount = 0;
nsCOMPtr<nsISHContainer> prevContainer(do_QueryInterface(aPrevEntry));
nsCOMPtr<nsISHContainer> nextContainer(do_QueryInterface(aNextEntry));
- if (!aParent || !prevContainer || !nextContainer) {
+ if (!prevContainer || !nextContainer) {
return NS_ERROR_FAILURE;
}
prevContainer->GetChildCount(&pcnt);
nextContainer->GetChildCount(&ncnt);
aParent->GetChildCount(&dsCount);
// Create an array for child docshells.
@@ -1739,17 +1695,17 @@ nsSHistory::CompareFrames(nsISHEntry* aP
break;
}
}
}
// Finally recursively call this method.
// This will either load a new page to shell or some subshell or
// do nothing.
- CompareFrames(pChild, nChild, dsChild, aLoadType, aIsFrameFound);
+ LoadDifferingEntries(pChild, nChild, dsChild, aLoadType, aDifferenceFound);
}
return result;
}
nsresult
nsSHistory::InitiateLoad(nsISHEntry* aFrameEntry, nsIDocShell* aFrameDS,
long aLoadType)
{
--- a/docshell/shistory/nsSHistory.h
+++ b/docshell/shistory/nsSHistory.h
@@ -48,19 +48,19 @@ public:
protected:
virtual ~nsSHistory();
friend class nsSHEnumerator;
friend class nsSHistoryObserver;
// Could become part of nsIWebNavigation
NS_IMETHOD GetTransactionAtIndex(int32_t aIndex, nsISHTransaction** aResult);
- nsresult CompareFrames(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
- nsIDocShell* aRootDocShell, long aLoadType,
- bool* aIsFrameFound);
+ nsresult LoadDifferingEntries(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
+ nsIDocShell* aRootDocShell, long aLoadType,
+ bool& aDifferenceFound);
nsresult InitiateLoad(nsISHEntry* aFrameEntry, nsIDocShell* aFrameDS,
long aLoadType);
NS_IMETHOD LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd);
#ifdef DEBUG
nsresult PrintHistory();
#endif