Merge autoland to mozilla-central r=merge a=merge

accessible/ipc/win/HandlerProvider.cpp

@@ -21,6 +21,7 @@
 #include "mozilla/Move.h"
 #include "mozilla/mscom/AgileReference.h"
 #include "mozilla/mscom/FastMarshaler.h"
+#include "mozilla/mscom/Interceptor.h"
 #include "mozilla/mscom/MainThreadInvoker.h"
 #include "mozilla/mscom/Ptr.h"
 #include "mozilla/mscom/StructStream.h"
@@ -97,7 +98,8 @@ HandlerProvider::GetHandler(NotNull<CLSID*> aHandlerClsid)
 }
 
 void
-HandlerProvider::GetAndSerializePayload(const MutexAutoLock&)
+HandlerProvider::GetAndSerializePayload(const MutexAutoLock&,
+    NotNull<mscom::IInterceptor*> aInterceptor)
 {
   MOZ_ASSERT(mscom::IsCurrentThreadMTA());
 
@@ -107,10 +109,11 @@ HandlerProvider::GetAndSerializePayload(const MutexAutoLock&)
 
   IA2Payload payload{};
 
-  if (!mscom::InvokeOnMainThread("HandlerProvider::BuildIA2Data",
-                                 this, &HandlerProvider::BuildIA2Data,
-                                 &payload.mData) ||
-      !payload.mData.mUniqueId) {
+  if (!mscom::InvokeOnMainThread("HandlerProvider::BuildInitialIA2Data",
+                                 this, &HandlerProvider::BuildInitialIA2Data,
+                                 aInterceptor,
+                                 &payload.mStaticData, &payload.mDynamicData) ||
+      !payload.mDynamicData.mUniqueId) {
     return;
   }
 
@@ -124,9 +127,10 @@ HandlerProvider::GetAndSerializePayload(const MutexAutoLock&)
 
   mSerializer = MakeUnique<mscom::StructToStream>(payload, &IA2Payload_Encode);
 
-  // Now that we have serialized payload, we should free any BSTRs that were
-  // allocated in BuildIA2Data.
-  ClearIA2Data(payload.mData);
+  // Now that we have serialized payload, we should clean up any
+  // BSTRs, interfaces, etc. fetched in BuildInitialIA2Data.
+  CleanupStaticIA2Data(payload.mStaticData);
+  CleanupDynamicIA2Data(payload.mDynamicData);
 }
 
 HRESULT
@@ -142,7 +146,7 @@ HandlerProvider::GetHandlerPayloadSize(NotNull<mscom::IInterceptor*> aIntercepto
 
   MutexAutoLock lock(mMutex);
 
-  GetAndSerializePayload(lock);
+  GetAndSerializePayload(lock, aInterceptor);
 
   if (!mSerializer || !(*mSerializer)) {
     // Failed payload serialization is non-fatal
@@ -182,7 +186,72 @@ private:
 };
 
 void
-HandlerProvider::BuildIA2Data(IA2Data* aOutIA2Data)
+HandlerProvider::BuildStaticIA2Data(
+  NotNull<mscom::IInterceptor*> aInterceptor,
+  StaticIA2Data* aOutData)
+{
+  MOZ_ASSERT(aOutData);
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(mTargetUnk);
+  MOZ_ASSERT(IsTargetInterfaceCacheable());
+
+  // Include interfaces the client is likely to request.
+  // This is cheap here and saves multiple cross-process calls later.
+  // These interfaces must be released in CleanupStaticIA2Data!
+
+  // If the target is already an IAccessible2, this pointer is redundant.
+  // However, the target might be an IAccessibleHyperlink, etc., in which
+  // case the client will almost certainly QI for IAccessible2.
+  HRESULT hr = aInterceptor->GetInterceptorForIID(NEWEST_IA2_IID,
+                                          (void**)&aOutData->mIA2);
+  if (FAILED(hr)) {
+    // IA2 should always be present, so something has
+    // gone very wrong if this fails.
+    aOutData->mIA2 = nullptr;
+    return;
+  }
+
+  // Some of these interfaces aren't present on all accessibles,
+  // so it's not a failure if these interfaces can't be fetched.
+  hr = aInterceptor->GetInterceptorForIID(IID_IEnumVARIANT,
+                                          (void**)&aOutData->mIEnumVARIANT);
+  if (FAILED(hr)) {
+    aOutData->mIEnumVARIANT = nullptr;
+  }
+
+  hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleHypertext2,
+                                          (void**)&aOutData->mIAHypertext);
+  if (FAILED(hr)) {
+    aOutData->mIAHypertext = nullptr;
+  }
+
+  hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleHyperlink,
+                                          (void**)&aOutData->mIAHyperlink);
+  if (FAILED(hr)) {
+    aOutData->mIAHyperlink = nullptr;
+  }
+
+  hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleTable,
+                                          (void**)&aOutData->mIATable);
+  if (FAILED(hr)) {
+    aOutData->mIATable = nullptr;
+  }
+
+  hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleTable2,
+                                          (void**)&aOutData->mIATable2);
+  if (FAILED(hr)) {
+    aOutData->mIATable2 = nullptr;
+  }
+
+  hr = aInterceptor->GetInterceptorForIID(IID_IAccessibleTableCell,
+                                          (void**)&aOutData->mIATableCell);
+  if (FAILED(hr)) {
+    aOutData->mIATableCell = nullptr;
+  }
+}
+
+void
+HandlerProvider::BuildDynamicIA2Data(DynamicIA2Data* aOutIA2Data)
 {
   MOZ_ASSERT(aOutIA2Data);
   MOZ_ASSERT(NS_IsMainThread());
@@ -203,7 +272,7 @@ HandlerProvider::BuildIA2Data(IA2Data* aOutIA2Data)
   };
 
   auto cleanup = [this, aOutIA2Data]() -> void {
-    ClearIA2Data(*aOutIA2Data);
+    CleanupDynamicIA2Data(*aOutIA2Data);
   };
 
   ExecuteWhen<decltype(hasFailed), decltype(cleanup)> onFail(hasFailed, cleanup);
@@ -246,6 +315,11 @@ HandlerProvider::BuildIA2Data(IA2Data* aOutIA2Data)
     return;
   }
 
+  hr = target->get_accDefaultAction(kChildIdSelf, &aOutIA2Data->mDefaultAction);
+  if (FAILED(hr)) {
+    return;
+  }
+
   hr = target->get_accChildCount(&aOutIA2Data->mChildCount);
   if (FAILED(hr)) {
     return;
@@ -284,6 +358,32 @@ HandlerProvider::BuildIA2Data(IA2Data* aOutIA2Data)
     return;
   }
 
+  RefPtr<IAccessibleAction> action;
+  // It is not an error if this fails.
+  hr = mTargetUnk.get()->QueryInterface(IID_IAccessibleAction,
+    getter_AddRefs(action));
+  if (SUCCEEDED(hr)) {
+    hr = action->nActions(&aOutIA2Data->mNActions);
+    if (FAILED(hr)) {
+      return;
+    }
+  }
+
+  RefPtr<IAccessibleTableCell> cell;
+  // It is not an error if this fails.
+  hr = mTargetUnk.get()->QueryInterface(IID_IAccessibleTableCell,
+    getter_AddRefs(cell));
+  if (SUCCEEDED(hr)) {
+    hr = cell->get_rowColumnExtents(&aOutIA2Data->mRowIndex,
+                                    &aOutIA2Data->mColumnIndex,
+                                    &aOutIA2Data->mRowExtent,
+                                    &aOutIA2Data->mColumnExtent,
+                                    &aOutIA2Data->mCellIsSelected);
+    if (FAILED(hr)) {
+      return;
+    }
+  }
+
   // NB: get_uniqueID should be the final property retrieved in this method,
   // as its presence is used to determine whether the rest of this data
   // retrieval was successful.
@@ -291,10 +391,57 @@ HandlerProvider::BuildIA2Data(IA2Data* aOutIA2Data)
 }
 
 void
-HandlerProvider::ClearIA2Data(IA2Data& aData)
+HandlerProvider::CleanupStaticIA2Data(StaticIA2Data& aData)
+{
+  // When CoMarshalInterface writes interfaces out to a stream, it AddRefs.
+  // Therefore, we must release our references after this.
+  if (aData.mIA2) {
+    aData.mIA2->Release();
+  }
+  if (aData.mIEnumVARIANT) {
+    aData.mIEnumVARIANT->Release();
+  }
+  if (aData.mIAHypertext) {
+    aData.mIAHypertext->Release();
+  }
+  if (aData.mIAHyperlink) {
+    aData.mIAHyperlink->Release();
+  }
+  if (aData.mIATable) {
+    aData.mIATable->Release();
+  }
+  if (aData.mIATable2) {
+    aData.mIATable2->Release();
+  }
+  if (aData.mIATableCell) {
+    aData.mIATableCell->Release();
+  }
+  ZeroMemory(&aData, sizeof(StaticIA2Data));
+}
+
+void
+HandlerProvider::CleanupDynamicIA2Data(DynamicIA2Data& aData)
 {
   ::VariantClear(&aData.mRole);
-  ZeroMemory(&aData, sizeof(IA2Data));
+  ZeroMemory(&aData, sizeof(DynamicIA2Data));
+}
+
+void
+HandlerProvider::BuildInitialIA2Data(
+  NotNull<mscom::IInterceptor*> aInterceptor,
+  StaticIA2Data* aOutStaticData,
+  DynamicIA2Data* aOutDynamicData)
+{
+  BuildStaticIA2Data(aInterceptor, aOutStaticData);
+  if (!aOutStaticData->mIA2) {
+    return;
+  }
+  BuildDynamicIA2Data(aOutDynamicData);
+  if (!aOutDynamicData->mUniqueId) {
+    // Building dynamic data failed, which means building the payload failed.
+    // However, we've already built static data, so we must clean this up.
+    CleanupStaticIA2Data(*aOutStaticData);
+  }
 }
 
 bool
@@ -407,12 +554,12 @@ HandlerProvider::put_HandlerControl(long aPid, IHandlerControl* aCtrl)
 }
 
 HRESULT
-HandlerProvider::Refresh(IA2Data* aOutData)
+HandlerProvider::Refresh(DynamicIA2Data* aOutData)
 {
   MOZ_ASSERT(mscom::IsCurrentThreadMTA());
 
-  if (!mscom::InvokeOnMainThread("HandlerProvider::BuildIA2Data",
-                                 this, &HandlerProvider::BuildIA2Data,
+  if (!mscom::InvokeOnMainThread("HandlerProvider::BuildDynamicIA2Data",
+                                 this, &HandlerProvider::BuildDynamicIA2Data,
                                  aOutData)) {
     return E_FAIL;
   }

accessible/ipc/win/HandlerProvider.h

@@ -54,16 +54,23 @@ public:
 
   // IGeckoBackChannel
   STDMETHODIMP put_HandlerControl(long aPid, IHandlerControl* aCtrl) override;
-  STDMETHODIMP Refresh(IA2Data* aOutData) override;
+  STDMETHODIMP Refresh(DynamicIA2Data* aOutData) override;
 
 private:
   ~HandlerProvider() = default;
 
   void SetHandlerControlOnMainThread(DWORD aPid,
                                      mscom::ProxyUniquePtr<IHandlerControl> aCtrl);
-  void GetAndSerializePayload(const MutexAutoLock&);
-  void BuildIA2Data(IA2Data* aOutIA2Data);
-  static void ClearIA2Data(IA2Data& aData);
+  void GetAndSerializePayload(const MutexAutoLock&,
+                              NotNull<mscom::IInterceptor*> aInterceptor);
+  void BuildStaticIA2Data(NotNull<mscom::IInterceptor*> aInterceptor,
+                          StaticIA2Data* aOutData);
+  void BuildDynamicIA2Data(DynamicIA2Data* aOutIA2Data);
+  void BuildInitialIA2Data(NotNull<mscom::IInterceptor*> aInterceptor,
+                           StaticIA2Data* aOutStaticData,
+                           DynamicIA2Data* aOutDynamicData);
+  static void CleanupStaticIA2Data(StaticIA2Data& aData);
+  static void CleanupDynamicIA2Data(DynamicIA2Data& aData);
   bool IsTargetInterfaceCacheable();
 
   Atomic<uint32_t>                  mRefCnt;

accessible/ipc/win/handler/AccessibleHandler.cpp

@@ -29,7 +29,11 @@
 #include "Accessible2_i.c"
 #include "Accessible2_2_i.c"
 #include "Accessible2_3_i.c"
+#include "AccessibleAction_i.c"
 #include "AccessibleHyperlink_i.c"
+#include "AccessibleTable_i.c"
+#include "AccessibleTable2_i.c"
+#include "AccessibleTableCell_i.c"
 
 namespace mozilla {
 namespace a11y {
@@ -63,6 +67,7 @@ AccessibleHandler::AccessibleHandler(IUnknown* aOuter, HRESULT* aResult)
   , mIA2PassThru(nullptr)
   , mServProvPassThru(nullptr)
   , mIAHyperlinkPassThru(nullptr)
+  , mIATableCellPassThru(nullptr)
   , mCachedData()
   , mCacheGen(0)
 {
@@ -130,6 +135,29 @@ AccessibleHandler::ResolveIAHyperlink()
   return hr;
 }
 
+HRESULT
+AccessibleHandler::ResolveIATableCell()
+{
+  if (mIATableCellPassThru) {
+    return S_OK;
+  }
+
+  RefPtr<IUnknown> proxy(GetProxy());
+  if (!proxy) {
+    return E_UNEXPECTED;
+  }
+
+  HRESULT hr = proxy->QueryInterface(IID_IAccessibleTableCell,
+    reinterpret_cast<void**>(&mIATableCellPassThru));
+  if (SUCCEEDED(hr)) {
+    // mIATableCellPassThru is a weak reference
+    // (see comments in AccesssibleHandler.h)
+    mIATableCellPassThru->Release();
+  }
+
+  return hr;
+}
+
 HRESULT
 AccessibleHandler::MaybeUpdateCachedData()
 {
@@ -147,7 +175,7 @@ AccessibleHandler::MaybeUpdateCachedData()
     return E_POINTER;
   }
 
-  return mCachedData.mGeckoBackChannel->Refresh(&mCachedData.mData);
+  return mCachedData.mGeckoBackChannel->Refresh(&mCachedData.mDynamicData);
 }
 
 HRESULT
@@ -210,6 +238,46 @@ AccessibleHandler::QueryHandlerInterface(IUnknown* aProxyUnknown, REFIID aIid,
     return S_OK;
   }
 
+  if (HasPayload()) {
+    // The proxy manager caches interfaces marshaled in the payload
+    // and returns them on QI without a cross-process call.
+    // However, it doesn't know about interfaces which don't exist.
+    // We can determine this from the payload.
+    if ((aIid == IID_IEnumVARIANT &&
+         !mCachedData.mStaticData.mIEnumVARIANT) ||
+        ((aIid == IID_IAccessibleText || aIid == IID_IAccessibleHypertext ||
+          aIid == IID_IAccessibleHypertext2) &&
+         !mCachedData.mStaticData.mIAHypertext) ||
+        ((aIid == IID_IAccessibleAction || aIid == IID_IAccessibleHyperlink) &&
+         !mCachedData.mStaticData.mIAHyperlink) ||
+        (aIid == IID_IAccessibleTable &&
+         !mCachedData.mStaticData.mIATable) ||
+        (aIid == IID_IAccessibleTable2 &&
+         !mCachedData.mStaticData.mIATable2) ||
+        (aIid == IID_IAccessibleTableCell &&
+         !mCachedData.mStaticData.mIATableCell)) {
+      // We already know this interface is not available, so don't query
+      // the proxy, thus avoiding a pointless cross-process call.
+      // If we return E_NOINTERFACE here, mscom::Handler will try the COM
+      // proxy. S_FALSE signals that the proxy should not be tried.
+      return S_FALSE;
+    }
+  }
+
+  if (aIid == IID_IAccessibleAction || aIid == IID_IAccessibleHyperlink) {
+    RefPtr<IAccessibleHyperlink> iaLink(
+      static_cast<IAccessibleHyperlink*>(this));
+    iaLink.forget(aOutInterface);
+    return S_OK;
+  }
+
+  if (aIid == IID_IAccessibleTableCell) {
+    RefPtr<IAccessibleTableCell> iaCell(
+      static_cast<IAccessibleTableCell*>(this));
+    iaCell.forget(aOutInterface);
+    return S_OK;
+  }
+
   if (aIid == IID_IAccessibleText || aIid == IID_IAccessibleHypertext ||
       aIid == IID_IAccessibleHypertext2) {
     RefPtr<IAccessibleHypertext2> textTearoff(new AccessibleTextTearoff(this));
@@ -241,9 +309,48 @@ AccessibleHandler::ReadHandlerPayload(IStream* aStream, REFIID aIid)
     return S_FALSE;
   }
 
-  if (!deserializer.Read(&mCachedData, &IA2Payload_Decode)) {
+  // QueryHandlerInterface might get called while we deserialize the payload,
+  // but that checks the interface pointers in the payload to determine what
+  // interfaces are available. Therefore, deserialize into a temporary struct
+  // and update mCachedData only after deserialization completes.
+  // The decoding functions can misbehave if their target memory is not zeroed
+  // beforehand, so ensure we do that.
+  IA2Payload newData{};
+  if (!deserializer.Read(&newData, &IA2Payload_Decode)) {
     return E_FAIL;
   }
+  mCachedData = newData;
+
+  // These interfaces have been aggregated into the proxy manager.
+  // The proxy manager will resolve these interfaces now on QI,
+  // so we can release these pointers.
+  // However, we don't null them out because we use their presence
+  // to determine whether the interface is available
+  // so as to avoid pointless cross-proc QI calls returning E_NOINTERFACE.
+  // Note that if pointers to other objects (in contrast to
+  // interfaces of *this* object) are added in future, we should not release
+  // those pointers.
+  if (mCachedData.mStaticData.mIA2) {
+    mCachedData.mStaticData.mIA2->Release();
+  }
+  if (mCachedData.mStaticData.mIEnumVARIANT) {
+    mCachedData.mStaticData.mIEnumVARIANT->Release();
+  }
+  if (mCachedData.mStaticData.mIAHypertext) {
+    mCachedData.mStaticData.mIAHypertext->Release();
+  }
+  if (mCachedData.mStaticData.mIAHyperlink) {
+    mCachedData.mStaticData.mIAHyperlink->Release();
+  }
+  if (mCachedData.mStaticData.mIATable) {
+    mCachedData.mStaticData.mIATable->Release();
+  }
+  if (mCachedData.mStaticData.mIATable2) {
+    mCachedData.mStaticData.mIATable2->Release();
+  }
+  if (mCachedData.mStaticData.mIATableCell) {
+    mCachedData.mStaticData.mIATableCell->Release();
+  }
 
   if (!mCachedData.mGeckoBackChannel) {
     return S_OK;
@@ -411,12 +518,12 @@ CopyBSTR(BSTR aSrc)
 
 #define GET_FIELD(member, assignTo) \
   { \
-    assignTo = mCachedData.mData.member; \
+    assignTo = mCachedData.mDynamicData.member; \
   }
 
 #define GET_BSTR(member, assignTo) \
   { \
-    assignTo = CopyBSTR(mCachedData.mData.member); \
+    assignTo = CopyBSTR(mCachedData.mDynamicData.member); \
   }
 
 /*** IAccessible ***/
@@ -547,7 +654,7 @@ AccessibleHandler::get_accRole(VARIANT varChild, VARIANT *pvarRole)
   }
 
   BEGIN_CACHE_ACCESS;
-  return ::VariantCopy(pvarRole, &mCachedData.mData.mRole);
+  return ::VariantCopy(pvarRole, &mCachedData.mDynamicData.mRole);
 }
 
 
@@ -919,7 +1026,7 @@ AccessibleHandler::get_uniqueID(long* uniqueID)
     }
     return mIA2PassThru->get_uniqueID(uniqueID);
   }
-  *uniqueID = mCachedData.mData.mUniqueId;
+  *uniqueID = mCachedData.mDynamicData.mUniqueId;
   return S_OK;
 }
 
@@ -1119,11 +1226,21 @@ AccessibleHandler::GetClassInfo(ITypeInfo** aOutTypeInfo)
 HRESULT
 AccessibleHandler::nActions(long* nActions)
 {
-  HRESULT hr = ResolveIAHyperlink();
-  if (FAILED(hr)) {
-    return hr;
+  if (!nActions) {
+    return E_INVALIDARG;
   }
-  return mIAHyperlinkPassThru->nActions(nActions);
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIAHyperlink();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIAHyperlinkPassThru->nActions(nActions);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mNActions, *nActions);
+  return S_OK;
 }
 
 HRESULT
@@ -1163,6 +1280,24 @@ AccessibleHandler::get_keyBinding(long actionIndex,
 HRESULT
 AccessibleHandler::get_name(long actionIndex, BSTR* name)
 {
+  if (!name) {
+    return E_INVALIDARG;
+  }
+
+  if (HasPayload()) {
+    if (actionIndex >= mCachedData.mDynamicData.mNActions) {
+      // Action does not exist.
+      return E_INVALIDARG;
+    }
+
+    if (actionIndex == 0) {
+      // same as accDefaultAction.
+      GET_BSTR(mDefaultAction, *name);
+      return S_OK;
+    }
+  }
+
+  // At this point, there's either no payload or actionIndex is > 0.
   HRESULT hr = ResolveIAHyperlink();
   if (FAILED(hr)) {
     return hr;
@@ -1232,6 +1367,172 @@ AccessibleHandler::get_valid(boolean* valid)
   return mIAHyperlinkPassThru->get_valid(valid);
 }
 
+/*** IAccessibleTableCell ***/
+
+HRESULT
+AccessibleHandler::get_columnExtent(long* nColumnsSpanned)
+{
+  if (!nColumnsSpanned) {
+    return E_INVALIDARG;
+  }
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIATableCell();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIATableCellPassThru->get_columnExtent(nColumnsSpanned);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mColumnExtent, *nColumnsSpanned);
+  return S_OK;
+}
+
+HRESULT
+AccessibleHandler::get_columnHeaderCells(IUnknown*** cellAccessibles,
+                                     long* nColumnHeaderCells)
+{
+  HRESULT hr = ResolveIATableCell();
+  if (FAILED(hr)) {
+    return hr;
+  }
+
+  return mIATableCellPassThru->get_columnHeaderCells(cellAccessibles,
+             nColumnHeaderCells);
+}
+
+HRESULT
+AccessibleHandler::get_columnIndex(long* columnIndex)
+{
+  if (!columnIndex) {
+    return E_INVALIDARG;
+  }
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIATableCell();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIATableCellPassThru->get_columnIndex(columnIndex);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mColumnIndex, *columnIndex);
+  return S_OK;
+}
+
+HRESULT
+AccessibleHandler::get_rowExtent(long* nRowsSpanned)
+{
+  if (!nRowsSpanned) {
+    return E_INVALIDARG;
+  }
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIATableCell();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIATableCellPassThru->get_rowExtent(nRowsSpanned);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mRowExtent, *nRowsSpanned);
+  return S_OK;
+}
+
+HRESULT
+AccessibleHandler::get_rowHeaderCells(IUnknown*** cellAccessibles,
+                                  long* nRowHeaderCells)
+{
+  HRESULT hr = ResolveIATableCell();
+  if (FAILED(hr)) {
+    return hr;
+  }
+
+  return mIATableCellPassThru->get_rowHeaderCells(cellAccessibles,
+             nRowHeaderCells);
+}
+
+HRESULT
+AccessibleHandler::get_rowIndex(long* rowIndex)
+{
+  if (!rowIndex) {
+    return E_INVALIDARG;
+  }
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIATableCell();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIATableCellPassThru->get_rowIndex(rowIndex);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mRowIndex, *rowIndex);
+  return S_OK;
+}
+
+HRESULT
+AccessibleHandler::get_isSelected(boolean* isSelected)
+{
+  if (!isSelected) {
+    return E_INVALIDARG;
+  }
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIATableCell();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIATableCellPassThru->get_isSelected(isSelected);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mCellIsSelected, *isSelected);
+  return S_OK;
+}
+
+HRESULT
+AccessibleHandler::get_rowColumnExtents(long* row, long* column,
+                                     long* rowExtents, long* columnExtents,
+                                     boolean* isSelected)
+{
+  if (!row || !column || !rowExtents || !columnExtents || !isSelected) {
+    return E_INVALIDARG;
+  }
+
+  if (!HasPayload()) {
+    HRESULT hr = ResolveIATableCell();
+    if (FAILED(hr)) {
+      return hr;
+    }
+    return mIATableCellPassThru->get_rowColumnExtents(row, column, rowExtents,
+               columnExtents, isSelected);
+  }
+
+  BEGIN_CACHE_ACCESS;
+  GET_FIELD(mRowIndex, *row);
+  GET_FIELD(mColumnIndex, *column);
+  GET_FIELD(mRowExtent, *rowExtents);
+  GET_FIELD(mColumnExtent, *columnExtents);
+  GET_FIELD(mCellIsSelected, *isSelected);
+  return S_OK;
+}
+
+HRESULT
+AccessibleHandler::get_table(IUnknown** table)
+{
+  HRESULT hr = ResolveIATableCell();
+  if (FAILED(hr)) {
+    return hr;
+  }
+
+  return mIATableCellPassThru->get_table(table);
+}
+
 } // namespace a11y
 } // namespace mozilla
 

accessible/ipc/win/handler/AccessibleHandler.h

@@ -36,6 +36,7 @@ import NEWEST_IA2_IDL;
 
 #include "Accessible2_3.h"
 #include "AccessibleHyperlink.h"
+#include "AccessibleTableCell.h"
 #include "Handler.h"
 #include "mozilla/mscom/StructStream.h"
 #include "mozilla/UniquePtr.h"
@@ -51,6 +52,7 @@ class AccessibleHandler final : public mscom::Handler
                               , public IServiceProvider
                               , public IProvideClassInfo
                               , public IAccessibleHyperlink
+                              , public IAccessibleTableCell
 {
 public:
   static HRESULT Create(IUnknown* aOuter, REFIID aIid, void** aOutInterface);
@@ -173,6 +175,21 @@ public:
   STDMETHODIMP get_endIndex(long* index) override;
   STDMETHODIMP get_valid(boolean* valid) override;
 
+  // IAccessibleTableCell
+  STDMETHODIMP get_columnExtent(long* nColumnsSpanned) override;
+  STDMETHODIMP get_columnHeaderCells(IUnknown*** cellAccessibles,
+                                     long* nColumnHeaderCells) override;
+  STDMETHODIMP get_columnIndex(long* columnIndex) override;
+  STDMETHODIMP get_rowExtent(long* nRowsSpanned) override;
+  STDMETHODIMP get_rowHeaderCells(IUnknown*** cellAccessibles,
+                                  long* nRowHeaderCells) override;
+  STDMETHODIMP get_rowIndex(long* rowIndex) override;
+  STDMETHODIMP get_isSelected(boolean* isSelected) override;
+  STDMETHODIMP get_rowColumnExtents(long* row, long* column,
+                                    long* rowExtents, long* columnExtents,
+                                    boolean* isSelected) override;
+  STDMETHODIMP get_table(IUnknown** table) override;
+
 private:
   AccessibleHandler(IUnknown* aOuter, HRESULT* aResult);
   virtual ~AccessibleHandler();
@@ -180,6 +197,7 @@ private:
   HRESULT ResolveIA2();
   HRESULT ResolveIDispatch();
   HRESULT ResolveIAHyperlink();
+  HRESULT ResolveIATableCell();
   HRESULT MaybeUpdateCachedData();
 
   RefPtr<IUnknown>                  mDispatchUnk;
@@ -206,6 +224,7 @@ private:
   NEWEST_IA2_INTERFACE*             mIA2PassThru;      // weak
   IServiceProvider*                 mServProvPassThru; // weak
   IAccessibleHyperlink*             mIAHyperlinkPassThru; // weak
+  IAccessibleTableCell*             mIATableCellPassThru; // weak
   IA2Payload                        mCachedData;
   UniquePtr<mscom::StructToStream>  mSerializer;
   uint32_t                          mCacheGen;

accessible/ipc/win/handler/HandlerData.idl

@@ -7,13 +7,31 @@
 #include "mozilla-config.h"
 #include "AccessibleHandler.h"
 
+import "oaidl.idl";
 import "ocidl.idl";
 import "ServProv.idl";
 
-import "AccessibleText.idl";
+import "Accessible2_3.idl";
+import "AccessibleHypertext2.idl";
+import "AccessibleHyperlink.idl";
+import "AccessibleTable.idl";
+import "AccessibleTable2.idl";
+import "AccessibleTableCell.idl";
 
-typedef struct _IA2Data
+typedef struct _StaticIA2Data
 {
+  NEWEST_IA2_INTERFACE* mIA2;
+  IEnumVARIANT* mIEnumVARIANT;
+  IAccessibleHypertext2* mIAHypertext;
+  IAccessibleHyperlink* mIAHyperlink;
+  IAccessibleTable* mIATable;
+  IAccessibleTable2* mIATable2;
+  IAccessibleTableCell* mIATableCell;
+} StaticIA2Data;
+
+typedef struct _DynamicIA2Data
+{
+  // From IAccessible/IAccessible2
   VARIANT           mRole;
   long              mState;
   long              mChildCount;
@@ -31,8 +49,17 @@ typedef struct _IA2Data
   BSTR              mValue;
   BSTR              mAttributes;
   IA2Locale         mIA2Locale;
+  // From IAccessibleAction
+  long              mNActions;
+  // From IAccessibleTableCell
+  long              mRowIndex;
+  long              mColumnIndex;
+  long              mRowExtent;
+  long              mColumnExtent;
+  boolean           mCellIsSelected;
+  // From IAccessible2
   long              mUniqueId;
-} IA2Data;
+} DynamicIA2Data;
 
 interface IGeckoBackChannel;
 
@@ -100,7 +127,8 @@ interface HandlerData
 {
   typedef struct _IA2Payload
   {
-    IA2Data mData;
+    StaticIA2Data mStaticData;
+    DynamicIA2Data mDynamicData;
     IGeckoBackChannel* mGeckoBackChannel;
   } IA2Payload;
 }
@@ -123,7 +151,7 @@ interface IHandlerControl : IUnknown
 interface IGeckoBackChannel : IUnknown
 {
   [propput] HRESULT HandlerControl([in] long aPid, [in] IHandlerControl* aCtrl);
-  HRESULT Refresh([out] IA2Data* aOutData);
+  HRESULT Refresh([out] DynamicIA2Data* aOutData);
 }
 
 [uuid(1e545f07-f108-4912-9471-546827a80983)]

browser/extensions/formautofill/FormAutofillContent.jsm

@@ -32,7 +32,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "InsecurePasswordUtils",
 
 const formFillController = Cc["@mozilla.org/satchel/form-fill-controller;1"]
                              .getService(Ci.nsIFormFillController);
-const {ADDRESSES_COLLECTION_NAME, CREDITCARDS_COLLECTION_NAME} = FormAutofillUtils;
+const {ADDRESSES_COLLECTION_NAME, CREDITCARDS_COLLECTION_NAME, FIELD_STATES} = FormAutofillUtils;
 
 // Register/unregister a constructor as a factory.
 function AutocompleteFactory() {}
@@ -101,6 +101,7 @@ AutofillProfileAutoCompleteSearch.prototype = {
     let focusedInput = formFillController.focusedInput;
     let info = FormAutofillContent.getInputDetails(focusedInput);
     let isAddressField = FormAutofillUtils.isAddressField(info.fieldName);
+    let isInputAutofilled = info.state == FIELD_STATES.AUTO_FILLED;
     let handler = FormAutofillContent.getFormHandler(focusedInput);
     let allFieldNames = handler.getAllFieldNames(focusedInput);
     let filledRecordGUID = handler.getFilledRecordGUID(focusedInput);
@@ -114,7 +115,8 @@ AutofillProfileAutoCompleteSearch.prototype = {
     //   - no profile can fill the currently-focused input.
     //   - the current form has already been populated.
     //   - (address only) less than 3 inputs are covered by all saved fields in the storage.
-    if (!searchPermitted || !savedFieldNames.has(info.fieldName) || filledRecordGUID || (isAddressField &&
+    if (!searchPermitted || !savedFieldNames.has(info.fieldName) ||
+        (!isInputAutofilled && filledRecordGUID) || (isAddressField &&
         allFieldNames.filter(field => savedFieldNames.has(field)).length < FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD)) {
       if (focusedInput.autocomplete == "off") {
         // Create a dummy AddressResult as an empty search result.
@@ -156,7 +158,7 @@ AutofillProfileAutoCompleteSearch.prototype = {
                                    info.fieldName,
                                    allFieldNames,
                                    adaptedRecords,
-                                   {});
+                                   {isInputAutofilled});
       } else {
         let isSecure = InsecurePasswordUtils.isFormSecure(handler.form);
 
@@ -164,7 +166,7 @@ AutofillProfileAutoCompleteSearch.prototype = {
                                       info.fieldName,
                                       allFieldNames,
                                       adaptedRecords,
-                                      {isSecure});
+                                      {isSecure, isInputAutofilled});
       }
       listener.onSearchResult(this, result);
       ProfileAutocomplete.lastProfileAutoCompleteResult = result;
@@ -511,6 +513,16 @@ var FormAutofillContent = {
     );
   },
 
+  clearForm() {
+    let focusedInput = formFillController.focusedInput || ProfileAutocomplete._lastAutoCompleteFocusedInput;
+    if (!focusedInput) {
+      return;
+    }
+
+    let formHandler = this.getFormHandler(focusedInput);
+    formHandler.clearPopulatedForm(focusedInput);
+  },
+
   previewProfile(doc) {
     let docWin = doc.ownerGlobal;
     let selectedIndex = ProfileAutocomplete._getSelectedIndex(docWin);
@@ -575,11 +587,13 @@ var FormAutofillContent = {
 
     let selectedIndex = ProfileAutocomplete._getSelectedIndex(e.target.ownerGlobal);
     let selectedRowStyle = lastAutoCompleteResult.getStyleAt(selectedIndex);
-    if (selectedRowStyle == "autofill-footer") {
-      focusedInput.addEventListener("DOMAutoComplete", () => {
+    focusedInput.addEventListener("DOMAutoComplete", () => {
+      if (selectedRowStyle == "autofill-footer") {
         Services.cpmm.sendAsyncMessage("FormAutofill:OpenPreferences");
-      }, {once: true});
-    }
+      } else if (selectedRowStyle == "autofill-clear-button") {
+        FormAutofillContent.clearForm();
+      }
+    }, {once: true});
   },
 };
 

browser/extensions/formautofill/FormAutofillHandler.jsm

@@ -27,6 +27,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "FormLikeFactory",
 this.log = null;
 FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);
 
+const {FIELD_STATES} = FormAutofillUtils;
+
 class FormAutofillSection {
   constructor(fieldDetails, winUtils) {
     this.address = {
@@ -55,11 +57,11 @@ class FormAutofillSection {
      */
     this._FIELD_STATE_ENUM = {
       // not themed
-      NORMAL: null,
+      [FIELD_STATES.NORMAL]: null,
       // highlighted
-      AUTO_FILLED: "-moz-autofill",
+      [FIELD_STATES.AUTO_FILLED]: "-moz-autofill",
       // highlighted && grey color text
-      PREVIEW: "-moz-autofill-preview",
+      [FIELD_STATES.PREVIEW]: "-moz-autofill-preview",
     };
 
     this.winUtils = winUtils;
@@ -390,7 +392,7 @@ class FormAutofillSection {
         if (element == focusedInput ||
             (element != focusedInput && !element.value)) {
           element.setUserInput(value);
-          this.changeFieldState(fieldDetail, "AUTO_FILLED");
+          this.changeFieldState(fieldDetail, FIELD_STATES.AUTO_FILLED);
           continue;
         }
       }
@@ -409,7 +411,7 @@ class FormAutofillSection {
           element.dispatchEvent(new element.ownerGlobal.Event("change", {bubbles: true}));
         }
         // Autofill highlight appears regardless if value is changed or not
-        this.changeFieldState(fieldDetail, "AUTO_FILLED");
+        this.changeFieldState(fieldDetail, FIELD_STATES.AUTO_FILLED);
       }
     }
   }
@@ -458,7 +460,7 @@ class FormAutofillSection {
         continue;
       }
       element.previewValue = value;
-      this.changeFieldState(fieldDetail, value ? "PREVIEW" : "NORMAL");
+      this.changeFieldState(fieldDetail, value ? FIELD_STATES.PREVIEW : FIELD_STATES.NORMAL);
     }
   }
 
@@ -483,11 +485,35 @@ class FormAutofillSection {
 
       // We keep the state if this field has
       // already been auto-filled.
-      if (fieldDetail.state === "AUTO_FILLED") {
+      if (fieldDetail.state == FIELD_STATES.AUTO_FILLED) {
         continue;
       }
 
-      this.changeFieldState(fieldDetail, "NORMAL");
+      this.changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
+    }
+  }
+
+  /**
+   * Clear value and highlight style of all filled fields.
+   *
+   * @param {Object} focusedInput
+   *        A focused input element for determining credit card or address fields.
+   */
+  clearPopulatedForm(focusedInput) {
+    let fieldDetails = this.getFieldDetailsByElement(focusedInput);
+    for (let fieldDetail of fieldDetails) {
+      let element = fieldDetail.elementWeakRef.get();
+      if (!element) {
+        log.warn(fieldDetail.fieldName, "is unreachable");
+        continue;
+      }
+
+      // Only reset value for input element.
+      if (fieldDetail.state == FIELD_STATES.AUTO_FILLED &&
+          element instanceof Ci.nsIDOMHTMLInputElement) {
+        element.setUserInput("");
+      }
+      this.changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
     }
   }
 
@@ -530,17 +556,17 @@ class FormAutofillSection {
 
   clearFieldState(focusedInput) {
     let fieldDetail = this.getFieldDetailByElement(focusedInput);
-    this.changeFieldState(fieldDetail, "NORMAL");
+    this.changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
     let targetSet = this._getTargetSet(focusedInput);
 
-    if (!targetSet.fieldDetails.some(detail => detail.state == "AUTO_FILLED")) {
+    if (!targetSet.fieldDetails.some(detail => detail.state == FIELD_STATES.AUTO_FILLED)) {
       targetSet.filledRecordGUID = null;
     }
   }
 
   resetFieldStates() {
     for (let fieldDetail of this._validDetails) {
-      this.changeFieldState(fieldDetail, "NORMAL");
+      this.changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
     }
     this.address.filledRecordGUID = null;
     this.creditCard.filledRecordGUID = null;
@@ -635,7 +661,7 @@ class FormAutofillSection {
 
         data[type].record[detail.fieldName] = value;
 
-        if (detail.state == "AUTO_FILLED") {
+        if (detail.state == FIELD_STATES.AUTO_FILLED) {
           data[type].untouchedFields.push(detail.fieldName);
         }
       });
@@ -871,6 +897,11 @@ class FormAutofillHandler {
     section.clearPreviewedFormFields(focusedInput);
   }
 
+  clearPopulatedForm(focusedInput) {
+    let section = this.getSectionByElement(focusedInput);
+    section.clearPopulatedForm(focusedInput);
+  }
+
   getFilledRecordGUID(focusedInput) {
     let section = this.getSectionByElement(focusedInput);
     return section.getFilledRecordGUID(focusedInput);
@@ -957,4 +988,3 @@ class FormAutofillHandler {
     return null;
   }
 }
-

browser/extensions/formautofill/FormAutofillUtils.jsm

@@ -30,6 +30,11 @@ const EDIT_ADDRESS_KEYWORDS = [
 ];
 const MANAGE_CREDITCARDS_KEYWORDS = ["manageCreditCardsTitle", "addNewCreditCardTitle", "showCreditCardsBtnLabel"];
 const EDIT_CREDITCARD_KEYWORDS = ["cardNumber", "nameOnCard", "cardExpires"];
+const FIELD_STATES = {
+  NORMAL: "NORMAL",
+  AUTO_FILLED: "AUTO_FILLED",
+  PREVIEW: "PREVIEW",
+};
 
 // The maximum length of data to be saved in a single field for preventing DoS
 // attacks that fill the user's hard drive(s).
@@ -54,6 +59,7 @@ this.FormAutofillUtils = {
   MANAGE_CREDITCARDS_KEYWORDS,
   EDIT_CREDITCARD_KEYWORDS,
   MAX_FIELD_VALUE_LENGTH,
+  FIELD_STATES,
 
   _fieldNameInfo: {
     "name": "name",

browser/extensions/formautofill/ProfileAutoCompleteResult.jsm

@@ -23,6 +23,7 @@ class ProfileAutoCompleteResult {
   constructor(searchString, focusedFieldName, allFieldNames, matchingProfiles, {
     resultCode = null,
     isSecure = true,
+    isInputAutofilled = false,
   }) {
     log.debug("Constructing new ProfileAutoCompleteResult:", [...arguments]);
 
@@ -41,6 +42,8 @@ class ProfileAutoCompleteResult {
     this.errorDescription = "";
     // The value used to determine whether the form is secure or not.
     this._isSecure = isSecure;
+    // The value to indicate whether the focused input has been autofilled or not.
+    this._isInputAutofilled = isInputAutofilled;
     // All fillable field names in the form including the field name of the currently-focused input.
     this._allFieldNames = [...this._matchingProfiles.reduce((fieldSet, curProfile) => {
       for (let field of Object.keys(curProfile)) {
@@ -93,18 +96,28 @@ class ProfileAutoCompleteResult {
   _generateLabels(focusedFieldName, allFieldNames, profiles) {}
 
   /**
-   * Retrieves a result
+   * Get the value of the result at the given index.
+   *
+   * Always return empty string for form autofill feature to suppress
+   * AutoCompleteController from autofilling, as we'll populate the
+   * fields on our own.
+   *
    * @param   {number} index The index of the result requested
    * @returns {string} The result at the specified index
    */
   getValueAt(index) {
     this._checkIndexBounds(index);
-    return this._popupLabels[index].primary;
+    return "";
   }
 
   getLabelAt(index) {
     this._checkIndexBounds(index);
-    return JSON.stringify(this._popupLabels[index]);
+
+    let label = this._popupLabels[index];
+    if (typeof label == "string") {
+      return label;
+    }
+    return JSON.stringify(label);
   }
 
   /**
@@ -127,6 +140,10 @@ class ProfileAutoCompleteResult {
     if (index == this.matchCount - 1) {
       return "autofill-footer";
     }
+    if (this._isInputAutofilled) {
+      return "autofill-clear-button";
+    }
+
     return "autofill-profile";
   }
 
@@ -237,6 +254,13 @@ class AddressResult extends ProfileAutoCompleteResult {
   }
 
   _generateLabels(focusedFieldName, allFieldNames, profiles) {
+    if (this._isInputAutofilled) {
+      return [
+        {primary: "", secondary: ""}, // Clear button
+        {primary: "", secondary: ""}, // Footer
+      ];
+    }
+
     // Skip results without a primary label.
     let labels = profiles.filter(profile => {
       return !!profile[focusedFieldName];
@@ -266,11 +290,6 @@ class AddressResult extends ProfileAutoCompleteResult {
 
     return labels;
   }
-
-  getValueAt(index) {
-    this._checkIndexBounds(index);
-    return "";
-  }
 }
 
 class CreditCardResult extends ProfileAutoCompleteResult {
@@ -337,6 +356,13 @@ class CreditCardResult extends ProfileAutoCompleteResult {
       return [FormAutofillUtils.stringBundle.formatStringFromName("insecureFieldWarningDescription", [brandName], 1)];
     }
 
+    if (this._isInputAutofilled) {
+      return [
+        {primary: "", secondary: ""}, // Clear button
+        {primary: "", secondary: ""}, // Footer
+      ];
+    }
+
     // Skip results without a primary label.
     let labels = profiles.filter(profile => {
       return !!profile[focusedFieldName];
@@ -363,35 +389,13 @@ class CreditCardResult extends ProfileAutoCompleteResult {
     return labels;
   }
 
-  // Always return empty string for credit card result. Since the decryption might
-  // be required of users' input, we have to suppress AutoCompleteController
-  // from filling encrypted data directly.
-  getValueAt(index) {
-    this._checkIndexBounds(index);
-    return "";
-  }
-
-  getLabelAt(index) {
-    this._checkIndexBounds(index);
-
-    let label = this._popupLabels[index];
-    if (typeof label == "string") {
-      return label;
-    }
-    return JSON.stringify(label);
-  }
-
   getStyleAt(index) {
     this._checkIndexBounds(index);
     if (!this._isSecure && insecureWarningEnabled) {
       return "autofill-insecureWarning";
     }
 
-    if (index == this.matchCount - 1) {
-      return "autofill-footer";
-    }
-
-    return "autofill-profile";
+    return super.getStyleAt(index);
   }
 
   getImageAt(index) {

browser/extensions/formautofill/content/FormAutofillFrameScript.js

@@ -45,6 +45,7 @@ var FormAutofillFrameScript = {
   init() {
     addEventListener("focusin", this);
     addMessageListener("FormAutofill:PreviewProfile", this);
+    addMessageListener("FormAutofill:ClearForm", this);
     addMessageListener("FormAutoComplete:PopupClosed", this);
     addMessageListener("FormAutoComplete:PopupOpened", this);
   },
@@ -88,6 +89,10 @@ var FormAutofillFrameScript = {
         FormAutofillContent.previewProfile(doc);
         break;
       }
+      case "FormAutofill:ClearForm": {
+        FormAutofillContent.clearForm();
+        break;
+      }
       case "FormAutoComplete:PopupClosed": {
         FormAutofillContent.onPopupClosed();
         chromeEventHandler.removeEventListener("keydown", FormAutofillContent._onKeyDown,
@@ -97,6 +102,7 @@ var FormAutofillFrameScript = {
       case "FormAutoComplete:PopupOpened": {
         chromeEventHandler.addEventListener("keydown", FormAutofillContent._onKeyDown,
                                             {capturing: true});
+        break;
       }
     }
   },

browser/extensions/formautofill/content/formautofill.css

@@ -4,7 +4,8 @@
 
 #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-profile"],
 #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-footer"],
-#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-insecureWarning"] {
+#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-insecureWarning"],
+#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-clear-button"] {
   display: block;
   margin: 0;
   padding: 0;
@@ -24,11 +25,15 @@
   -moz-binding: url("chrome://formautofill/content/formautofill.xml#autocomplete-creditcard-insecure-field");
 }
 
+#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-clear-button"] {
+  -moz-binding: url("chrome://formautofill/content/formautofill.xml#autocomplete-profile-listitem-clear-button");
+}
 /* Treat @collpased="true" as display: none similar to how it is for XUL elements.
  * https://developer.mozilla.org/en-US/docs/Web/CSS/visibility#Values */
 #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-profile"][collapsed="true"],
 #PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-footer"][collapsed="true"],
-#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-insecureWarning"][collapsed="true"] {
+#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-insecureWarning"][collapsed="true"],
+#PopupAutoComplete > richlistbox > richlistitem[originaltype="autofill-clear-button"][collapsed="true"] {
   display: none;
 }
 

browser/extensions/formautofill/content/formautofill.xml

@@ -153,7 +153,7 @@
       <div anonid="autofill-footer" class="autofill-item-box autofill-footer">
         <div anonid="autofill-warning" class="autofill-footer-row autofill-warning">
         </div>
-        <div anonid="autofill-option-button" class="autofill-footer-row autofill-option-button">
+        <div anonid="autofill-option-button" class="autofill-footer-row autofill-button">
         </div>
       </div>
     </xbl:content>
@@ -314,4 +314,49 @@
     </implementation>
   </binding>
 
+  <binding id="autocomplete-profile-listitem-clear-button" extends="chrome://formautofill/content/formautofill.xml#autocomplete-profile-listitem-base">
+    <xbl:content xmlns="http://www.w3.org/1999/xhtml">
+      <div anonid="autofill-item-box" class="autofill-item-box autofill-footer">
+        <div anonid="autofill-clear-button" class="autofill-footer-row autofill-button"></div>
+      </div>
+    </xbl:content>
+
+    <handlers>
+      <handler event="click" button="0"><![CDATA[
+        /* global Cu */
+        let {AutoCompletePopup} = Cu.import("resource://gre/modules/AutoCompletePopup.jsm", {});
+
+        AutoCompletePopup.sendMessageToBrowser("FormAutofill:ClearForm");
+      ]]></handler>
+    </handlers>
+
+    <implementation implements="nsIDOMXULSelectControlItemElement">
+      <constructor>
+      <![CDATA[
+        this._itemBox = document.getAnonymousElementByAttribute(
+          this, "anonid", "autofill-item-box"
+        );
+        this._clearBtn = document.getAnonymousElementByAttribute(
+          this, "anonid", "autofill-clear-button"
+        );
+
+        this._adjustAcItem();
+      ]]>
+      </constructor>
+
+      <method name="_adjustAcItem">
+        <body>
+        <![CDATA[
+          this._adjustAutofillItemLayout();
+          this.setAttribute("formautofillattached", "true");
+
+          let clearFormBtnLabel = this._stringBundle.GetStringFromName("clearFormBtnLabel");
+          this._clearBtn.textContent = clearFormBtnLabel;
+        ]]>
+        </body>
+      </method>
+
+    </implementation>
+  </binding>
+
 </bindings>

browser/extensions/formautofill/locales/en-US/formautofill.properties

@@ -78,6 +78,9 @@ phishingWarningMessage2 = Autofills %S
 # LOCALIZATION NOTE (insecureFieldWarningDescription): %S is brandShortName. This string is used in drop down
 # suggestion when users try to autofill credit card on an insecure website (without https).
 insecureFieldWarningDescription = %S has detected an insecure site. Form Autofill is temporarily disabled.
+# LOCALIZATION NOTE (clearFormBtnLabel): Label for the button in the dropdown menu that used to clear the populated
+# form.
+clearFormBtnLabel = Clear Form
 
 # LOCALIZATION NOTE (autofillAddressesCheckbox): Label for the checkbox that enables autofilling addresses.
 autofillAddressesCheckbox = Autofill addresses

browser/extensions/formautofill/skin/shared/autocomplete-item.css

@@ -10,7 +10,8 @@ xul|richlistitem[originaltype="autofill-profile"][selected="true"] > .autofill-i
   background-color: #F2F2F2;
 }
 
-xul|richlistitem[originaltype="autofill-footer"][selected="true"] > .autofill-item-box > .autofill-option-button {
+xul|richlistitem[originaltype="autofill-footer"][selected="true"] > .autofill-item-box > .autofill-button,
+xul|richlistitem[originaltype="autofill-clear-button"][selected="true"] > .autofill-item-box > .autofill-button {
   background-color: #DCDCDE;
 }
 
@@ -27,14 +28,14 @@ xul|richlistitem[originaltype="autofill-insecureWarning"] {
   --label-text-color: #262626;
   --comment-text-color: #646464;
   --warning-text-color: #646464;
-  --option-btn-text-color: -moz-FieldText;
+  --btn-text-color: -moz-FieldText;
 
   --default-font-size: 12;
   --label-affix-font-size: 10;
   --label-font-size: 12;
   --comment-font-size: 10;
   --warning-font-size: 10;
-  --option-btn-font-size: 11;
+  --btn-font-size: 11;
 }
 
 .autofill-item-box[size="small"] {
@@ -148,13 +149,13 @@ xul|richlistitem[originaltype="autofill-insecureWarning"] {
   font-size: calc(var(--warning-font-size) / var(--default-font-size) * 1em);
 }
 
-.autofill-footer > .autofill-option-button {
+.autofill-footer > .autofill-button {
   box-sizing: border-box;
   padding: 0 10px;
   min-height: 40px;
   background-color: #EDEDED;
-  font-size: calc(var(--option-btn-font-size) / var(--default-font-size) * 1em);
-  color: var(--option-btn-text-color);
+  font-size: calc(var(--btn-font-size) / var(--default-font-size) * 1em);
+  color: var(--btn-text-color);
   text-align: center;
 }
 

browser/extensions/formautofill/test/mochitest/formautofill_common.js

@@ -5,6 +5,7 @@
 "use strict";
 
 let formFillChromeScript;
+let defaultTextColor;
 let expectingPopup = null;
 
 const {FormAutofillUtils} = SpecialPowers.Cu.import("resource://formautofill/FormAutofillUtils.jsm");
@@ -66,6 +67,62 @@ function clickOnElement(selector) {
   SimpleTest.executeSoon(() => element.click());
 }
 
+// The equivalent helper function to getAdaptedProfiles in FormAutofillHandler.jsm that
+// transforms the given profile to expected filled profile.
+function _getAdaptedProfile(profile) {
+  const adaptedProfile = Object.assign({}, profile);
+
+  if (profile["street-address"]) {
+    adaptedProfile["street-address"] = FormAutofillUtils.toOneLineAddress(profile["street-address"]);
+  }
+
+  return adaptedProfile;
+}
+
+// We could not get ManuallyManagedState of element now, so directly check if
+// filter and text color style are applied.
+function checkFieldHighlighted(elem, expectedValue) {
+  const computedStyle = window.getComputedStyle(elem);
+  const isHighlighteApplied = computedStyle.getPropertyValue("filter") !== "none";
+
+  is(isHighlighteApplied, expectedValue, `Checking #${elem.id} highlight style`);
+}
+
+function checkFieldPreview(elem, expectedValue) {
+  const computedStyle = window.getComputedStyle(elem);
+  const isTextColorApplied = computedStyle.getPropertyValue("color") !== defaultTextColor;
+
+  is(SpecialPowers.wrap(elem).previewValue, expectedValue, `Checking #${elem.id} previewValue`);
+  is(isTextColorApplied, !!expectedValue, `Checking #${elem.id} preview style`);
+}
+
+function checkFieldValue(elem, expectedValue) {
+  if (typeof elem === "string") {
+    elem = document.querySelector(elem);
+  }
+  is(elem.value, String(expectedValue), "Checking " + elem.id + " field");
+}
+
+function triggerAutofillAndCheckProfile(profile) {
+  const adaptedProfile = _getAdaptedProfile(profile);
+  const promises = [];
+
+  for (const [fieldName, value] of Object.entries(adaptedProfile)) {
+    const element = document.getElementById(fieldName);
+    const expectingEvent = document.activeElement == element ? "DOMAutoComplete" : "change";
+    const checkFieldAutofilled = Promise.all([
+      new Promise(resolve => element.addEventListener("input", resolve, {once: true})),
+      new Promise(resolve => element.addEventListener(expectingEvent, resolve, {once: true})),
+    ]).then(() => checkFieldValue(element, value));
+
+    promises.push(checkFieldAutofilled);
+  }
+  // Press return key and trigger form autofill.
+  doKey("return");
+
+  return Promise.all(promises);
+}
+
 async function onStorageChanged(type) {
   info(`expecting the storage changed: ${type}`);
   return new Promise(resolve => {
@@ -183,6 +240,16 @@ function initPopupListener() {
   registerPopupShownListener(popupShownListener);
 }
 
+async function triggerPopupAndHoverItem(fieldSelector, selectIndex) {
+  await focusAndWaitForFieldsIdentified(fieldSelector);
+  doKey("down");
+  await expectPopup();
+  for (let i = 0; i <= selectIndex; i++) {
+    doKey("down");
+  }
+  await notifySelectedIndex(selectIndex);
+}
+
 function formAutoFillCommonSetup() {
   let chromeURL = SimpleTest.getTestFileURL("formautofill_parent_utils.js");
   formFillChromeScript = SpecialPowers.loadChromeScript(chromeURL);
@@ -201,6 +268,11 @@ function formAutoFillCommonSetup() {
     formFillChromeScript.destroy();
     expectingPopup = null;
   });
+
+  document.addEventListener("DOMContentLoaded", function() {
+    defaultTextColor = window.getComputedStyle(document.querySelector("input"))
+      .getPropertyValue("color");
+  }, {once: true});
 }
 
 formAutoFillCommonSetup();

browser/extensions/formautofill/test/mochitest/mochitest.ini

@@ -11,6 +11,7 @@ support-files =
 [test_basic_autocomplete_form.html]
 [test_basic_creditcard_autocomplete_form.html]
 scheme=https
+[test_clear_form.html]
 [test_creditcard_autocomplete_off.html]
 scheme=https
 [test_form_changes.html]

browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html

@@ -33,52 +33,6 @@ let MOCK_STORAGE = [{
   "address-level1": "CA",
 }];
 
-function checkElementFilled(element, expectedvalue) {
-  return [
-    new Promise(resolve => {
-      element.addEventListener("input", function onInput() {
-        ok(true, "Checking " + element.name + " field fires input event");
-        resolve();
-      }, {once: true});
-    }),
-    new Promise(resolve => {
-      element.addEventListener("change", function onChange() {
-        ok(true, "Checking " + element.name + " field fires change event");
-        is(element.value, expectedvalue, "Checking " + element.name + " field");
-        resolve();
-      }, {once: true});
-    }),
-  ];
-}
-
-function checkAutoCompleteInputFilled(element, expectedvalue) {
-  return new Promise(resolve => {
-    element.addEventListener("DOMAutoComplete", function onChange() {
-      is(element.value, expectedvalue, "Checking " + element.name + " field");
-      resolve();
-    }, {once: true});
-  });
-}
-
-function checkFormFilled(address) {
-  info("expecting form filled");
-  let promises = [];
-  for (let prop in address) {
-    let element = document.getElementById(prop);
-    if (document.activeElement == element) {
-      promises.push(checkAutoCompleteInputFilled(element, address[prop]));
-    } else {
-      let converted = address[prop];
-      if (prop == "street-address") {
-        converted = FormAutofillUtils.toOneLineAddress(converted);
-      }
-      promises.push(...checkElementFilled(element, converted));
-    }
-  }
-  doKey("return");
-  return Promise.all(promises);
-}
-
 async function setupAddressStorage() {
   await addAddress(MOCK_STORAGE[0]);
   await addAddress(MOCK_STORAGE[1]);
@@ -198,12 +152,12 @@ add_task(async function check_fields_after_form_autofill() {
     })
   ).slice(1));
   doKey("down");
-  await checkFormFilled(MOCK_STORAGE[1]);
+  await triggerAutofillAndCheckProfile(MOCK_STORAGE[1]);
 });
 
 // Fallback to history search after autofill address.
 add_task(async function check_fallback_after_form_autofill() {
-  await setInput("#tel", "");
+  await setInput("#tel", "", true);
   doKey("down");
   await expectPopup();
   checkMenuEntries(["+1234567890"], false);

browser/extensions/formautofill/test/mochitest/test_basic_creditcard_autocomplete_form.html

@@ -36,44 +36,6 @@ const reducedMockRecord = {
   "cc-number": "1234123456785678",
 };
 
-function checkElementFilled(element, expectedvalue) {
-  const focusedElem = document.activeElement;
-  const promises = [];
-
-  promises.push(new Promise(resolve => {
-    element.addEventListener("input", function onInput() {
-      ok(true, "Checking " + element.name + " field fires input event");
-      resolve();
-    }, {once: true});
-  }));
-
-  // Don't expect that focused input will receive "change" event since focus never move away.
-  if (element !== focusedElem) {
-    promises.push(new Promise(resolve => {
-      element.addEventListener("change", function onChange() {
-        ok(true, "Checking " + element.name + " field fires change event");
-        is(element.value, expectedvalue, "Checking " + element.name + " field");
-        resolve();
-      }, {once: true});
-    }));
-  }
-
-  return promises;
-}
-
-function checkFormFilled(creditCard) {
-  info("expecting form filled");
-  let promises = [];
-  for (let prop in creditCard) {
-    let element = document.getElementById(prop);
-    let converted = String(creditCard[prop]); // Convert potential number to string
-
-    promises.push(...checkElementFilled(element, converted));
-  }
-  doKey("return");
-  return Promise.all(promises);
-}
-
 async function setupCreditCardStorage() {
   await addCreditCard(MOCK_STORAGE[0]);
   await addCreditCard(MOCK_STORAGE[1]);
@@ -199,12 +161,12 @@ add_task(async function check_fields_after_form_autofill() {
   })));
 
   doKey("down");
-  await checkFormFilled(MOCK_STORAGE[1]);
+  await triggerAutofillAndCheckProfile(MOCK_STORAGE[1]);
 });
 
 // Fallback to history search after autofill address.
 add_task(async function check_fallback_after_form_autofill() {
-  await setInput("#cc-name", "");
+  await setInput("#cc-name", "", true);
   doKey("down");
   await expectPopup();
   checkMenuEntries(["John Smith"], false);

browser/extensions/formautofill/test/mochitest/test_clear_form.html

@@ -0,0 +1,90 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test form autofill - clear form button</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="formautofill_common.js"></script>
+  <script type="text/javascript" src="satchel_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+Form autofill test: clear form button
+
+<script>
+/* import-globals-from ../../../../../testing/mochitest/tests/SimpleTest/SpawnTask.js */
+/* import-globals-from ../../../../../toolkit/components/satchel/test/satchel_common.js */
+/* import-globals-from formautofill_common.js */
+
+"use strict";
+
+const MOCK_STORAGE = [{
+  organization: "Sesame Street",
+  "street-address": "123 Sesame Street.",
+  tel: "+13453453456",
+}, {
+  organization: "Mozilla",
+  "street-address": "331 E. Evelyn Avenue",
+}, {
+  organization: "Tel org",
+  tel: "+12223334444",
+}];
+
+initPopupListener();
+
+add_task(async function setup_storage() {
+  await addAddress(MOCK_STORAGE[0]);
+  await addAddress(MOCK_STORAGE[1]);
+  await addAddress(MOCK_STORAGE[2]);
+});
+
+function checkIsFormCleared(patch = {}) {
+  const form = document.getElementById("form1");
+
+  for (const elem of form.elements) {
+    const expectedValue = patch[elem.id] || "";
+    is(elem.value, expectedValue, "checking value");
+    checkFieldHighlighted(elem, false);
+    checkFieldPreview(elem, "");
+  }
+}
+
+add_task(async function simple_clear() {
+  await triggerPopupAndHoverItem("#organization", 0);
+  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
+
+  await triggerPopupAndHoverItem("#tel", 0);
+  doKey("return");
+  checkIsFormCleared();
+});
+
+add_task(async function clear_modified_form() {
+  await triggerPopupAndHoverItem("#organization", 0);
+  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
+
+  await setInput("#tel", "+1111111111", true);
+
+  await triggerPopupAndHoverItem("#street-address", 0);
+  doKey("return");
+  checkIsFormCleared({tel: "+1111111111"});
+});
+</script>
+
+<p id="display"></p>
+
+<div id="content">
+
+  <form id="form1">
+    <p>This is a basic form.</p>
+    <p><label>organization: <input id="organization" autocomplete="organization"></label></p>
+    <p><label>streetAddress: <input id="street-address" autocomplete="street-address"></label></p>
+    <p><label>tel: <input id="tel" autocomplete="tel"></label></p>
+    <p><label>country: <input id="country" autocomplete="country"></label></p>
+  </form>
+
+</div>
+
+<pre id="test"></pre>
+</body>
+</html>

browser/extensions/formautofill/test/mochitest/test_formautofill_preview_highlight.html

@@ -19,7 +19,6 @@ Form autofill test: preview and highlight
 
 "use strict";
 
-let defaultTextColor;
 const MOCK_STORAGE = [{
   organization: "Sesame Street",
   "street-address": "123 Sesame Street.",
@@ -32,66 +31,21 @@ const MOCK_STORAGE = [{
   tel: "+12223334444",
 }];
 
-// We could not get ManuallyManagedState of element now, so directly check if
-// filter and text color style are applied.
-function checkFieldPreview(elem, expectedText) {
-  const computedStyle = window.getComputedStyle(elem);
-  const isStyleApplied = computedStyle.getPropertyValue("filter") !== "none" &&
-                         computedStyle.getPropertyValue("color") !== defaultTextColor;
+function checkFormFieldsStyle(profile, isPreviewing = true) {
+  const elems = document.querySelectorAll("input, select");
 
-  is(SpecialPowers.wrap(elem).previewValue, expectedText, `Checking #${elem.id} previewValue`);
-  is(isStyleApplied, !!expectedText, `Checking #${elem.id} preview style`);
-}
+  for (const elem of elems) {
+    const fillableValue = profile && profile[elem.id];
+    const previewValue = isPreviewing && fillableValue || "";
 
-function checkFilledFieldHighlight(elem, expectedValue) {
-  const computedStyle = window.getComputedStyle(elem);
-  const isStyleApplied = computedStyle.getPropertyValue("filter") !== "none" &&
-                         computedStyle.getPropertyValue("color") === defaultTextColor;
-
-  is(SpecialPowers.wrap(elem).previewValue, "", `Checking #${elem.id} filled previewValue`);
-  is(isStyleApplied, expectedValue, `Checking #${elem.id} filled style`);
-}
-
-function checkFormPreviewFields(previewingAddress) {
-  const inputs = document.querySelectorAll("input");
-
-  for (const input of inputs) {
-    const previewValue = previewingAddress && previewingAddress[input.id] || "";
-
-    checkFieldPreview(input, previewValue);
+    checkFieldHighlighted(elem, !!fillableValue);
+    checkFieldPreview(elem, previewValue);
   }
 }
 
-function checkFormFilledFields(address) {
-  const inputs = document.querySelectorAll("input");
-
-  for (const input of inputs) {
-    const isFilledByAutofill = !!address[input.id];
-
-    checkFilledFieldHighlight(input, isFilledByAutofill);
-  }
-}
-
-function confirmAllFieldsFilled(address) {
-  info("expecting form filled");
-  const pendingPromises = [];
-
-  for (const prop in address) {
-    const element = document.getElementById(prop);
-
-    pendingPromises.push(new Promise(resolve => {
-      element.addEventListener("change", resolve, {once: true});
-    }));
-  }
-
-  return Promise.all(pendingPromises);
-}
-
 initPopupListener();
 
 add_task(async function setup_storage() {
-  defaultTextColor = window.getComputedStyle(document.querySelector("input")).getPropertyValue("color");
-
   await addAddress(MOCK_STORAGE[0]);
   await addAddress(MOCK_STORAGE[1]);
   await addAddress(MOCK_STORAGE[2]);
@@ -102,42 +56,31 @@ add_task(async function check_preview() {
 
   doKey("down");
   await expectPopup();
-  checkFormPreviewFields();
+  checkFormFieldsStyle(null);
 
   for (let i = 0; i < MOCK_STORAGE.length; i++) {
     doKey("down");
     await notifySelectedIndex(i);
-    checkFormPreviewFields(MOCK_STORAGE[i]);
+    checkFormFieldsStyle(MOCK_STORAGE[i]);
   }
 
   // Navigate to the footer
   doKey("down");
   await notifySelectedIndex(MOCK_STORAGE.length);
-  checkFormPreviewFields();
+  checkFormFieldsStyle(null);
 
   doKey("down");
   await notifySelectedIndex(-1);
-  checkFormPreviewFields();
+  checkFormFieldsStyle(null);
 
   focusedInput.blur();
 });
 
 add_task(async function check_filled_highlight() {
-  const focusedInput = await setInput("#organization", "");
-
-  doKey("down");
-  await expectPopup();
-
-  doKey("down");
-  await notifySelectedIndex(0);
-  const waitForFilled = confirmAllFieldsFilled(MOCK_STORAGE[0]);
-
+  await triggerPopupAndHoverItem("#organization", 0);
   // filled 1st address
-  doKey("return");
-  // blur to fire off change event from focusedInput
-  focusedInput.blur();
-  await waitForFilled;
-  checkFormFilledFields(MOCK_STORAGE[0]);
+  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
+  checkFormFieldsStyle(MOCK_STORAGE[0], false);
 });
 
 </script>

browser/modules/ContextMenu.jsm

@@ -541,13 +541,13 @@ class ContextMenu {
     let contentDisposition = null;
     if (aEvent.target.nodeType == Ci.nsIDOMNode.ELEMENT_NODE &&
         aEvent.target instanceof Ci.nsIImageLoadingContent &&
-        aEvent.target.currentURI) {
+        aEvent.target.currentRequestFinalURI) {
       disableSetDesktopBg = this._disableSetDesktopBackground(aEvent.target);
 
       try {
         let imageCache = Cc["@mozilla.org/image/tools;1"].getService(Ci.imgITools)
                                                          .getImgCacheForDocument(doc);
-        let props = imageCache.findEntryProperties(aEvent.target.currentURI, doc);
+        let props = imageCache.findEntryProperties(aEvent.target.currentRequestFinalURI, doc);
 
         try {
           contentType = props.get("type", Ci.nsISupportsCString).data;
@@ -810,7 +810,7 @@ class ContextMenu {
     // nsDocumentViewer::GetInImage. Make sure to update both if this is
     // changed.
     if (context.target instanceof Ci.nsIImageLoadingContent &&
-        context.target.currentURI) {
+        context.target.currentRequestFinalURI) {
       context.onImage = true;
 
       context.imageInfo = {
@@ -832,7 +832,7 @@ class ContextMenu {
         context.onCompletedImage = true;
       }
 
-      context.mediaURL = context.target.currentURI.spec;
+      context.mediaURL = context.target.currentRequestFinalURI.spec;
 
       const descURL = context.target.getAttribute("longdesc");
 

browser/themes/linux/syncedtabs/sidebar.css

@@ -35,15 +35,15 @@ html {
   height: 16px;
 }
 
-.search-box > .textbox-input-box > .textbox-input {
+.search-box > .textbox-input-box {
   background-image: url(chrome://global/skin/icons/search-textbox.svg);
   background-repeat: no-repeat;
   background-size: 12px 12px;
   background-position: left center;
-  text-indent: 14px;
+  padding-inline-start: 14px;
 }
 
-.search-box > .textbox-input-box > .textbox-input:-moz-locale-dir(rtl) {
+.search-box > .textbox-input-box:-moz-locale-dir(rtl) {
   background-position: right center;
 }
 

browser/themes/shared/urlbar-searchbar.inc.css

@@ -45,6 +45,11 @@
   color: var(--url-and-searchbar-color, black);
 }
 
+#urlbar:not([focused="true"]):-moz-lwtheme,
+#navigator-toolbox .searchbar-textbox:not([focused="true"]):-moz-lwtheme {
+  border-color: var(--url-and-searchbar-border-color, hsla(240,5%,5%,.25));
+}
+
 #urlbar:-moz-lwtheme:hover,
 #urlbar:-moz-lwtheme[focused="true"],
 #navigator-toolbox .searchbar-textbox:-moz-lwtheme:hover,

browser/themes/windows/syncedtabs/sidebar.css

@@ -27,15 +27,15 @@
   color: -moz-FieldText;
 }
 
-.search-box > .textbox-input-box > .textbox-input {
+.search-box > .textbox-input-box {
   background-image: url(chrome://global/skin/icons/search-textbox.svg);
   background-repeat: no-repeat;
   background-size: 12px 12px;
   background-position: left center;
-  text-indent: 14px;
+  padding-inline-start: 14px;
 }
 
-.search-box > .textbox-input-box > .textbox-input:-moz-locale-dir(rtl) {
+.search-box > .textbox-input-box:-moz-locale-dir(rtl) {
   background-position: right center;
 }
 

caps/NullPrincipalURI.cpp

@@ -311,6 +311,20 @@ NullPrincipalURI::CloneWithNewRef(const nsACString& newRef, nsIURI** _newURI)
   return Clone(_newURI);
 }
 
+NS_IMPL_ISUPPORTS(NullPrincipalURI::Mutator, nsIURISetters, nsIURIMutator)
+
+NS_IMETHODIMP
+NullPrincipalURI::Mutate(nsIURIMutator** aMutator)
+{
+    RefPtr<NullPrincipalURI::Mutator> mutator = new NullPrincipalURI::Mutator();
+    nsresult rv = mutator->InitFromURI(this);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+    mutator.forget(aMutator);
+    return NS_OK;
+}
+
 NS_IMETHODIMP
 NullPrincipalURI::Equals(nsIURI* aOther, bool* _equals)
 {

caps/NullPrincipalURI.h

@@ -19,6 +19,7 @@
 #include "mozilla/MemoryReporting.h"
 #include "NullPrincipal.h"
 #include "nsID.h"
+#include "nsIURIMutator.h"
 
 // {51fcd543-3b52-41f7-b91b-6b54102236e6}
 #define NS_NULLPRINCIPALURI_IMPLEMENTATION_CID \
@@ -38,14 +39,11 @@ public:
   virtual size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
   virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override;
 
-  // NB: This constructor exists only for deserialization.  Everyone
-  // else should call Create.
-  NullPrincipalURI();
-
   // Returns null on failure.
   static already_AddRefed<NullPrincipalURI> Create();
 
 private:
+  NullPrincipalURI();
   NullPrincipalURI(const NullPrincipalURI& aOther);
 
   ~NullPrincipalURI() {}
@@ -53,6 +51,44 @@ private:
   nsresult Init();
 
   nsAutoCStringN<NSID_LENGTH> mPath;
+
+public:
+  class Mutator
+      : public nsIURIMutator
+      , public BaseURIMutator<NullPrincipalURI>
+  {
+    NS_DECL_ISUPPORTS
+    NS_FORWARD_SAFE_NSIURISETTERS(mURI)
+
+    NS_IMETHOD Deserialize(const mozilla::ipc::URIParams& aParams) override
+    {
+      return InitFromIPCParams(aParams);
+    }
+
+    NS_IMETHOD Read(nsIObjectInputStream* aStream) override
+    {
+      return NS_ERROR_NOT_IMPLEMENTED;
+    }
+
+    NS_IMETHOD Finalize(nsIURI** aURI) override
+    {
+      mURI.forget(aURI);
+      return NS_OK;
+    }
+
+    NS_IMETHOD SetSpec(const nsACString & aSpec) override
+    {
+      return NS_ERROR_NOT_IMPLEMENTED;
+    }
+
+    explicit Mutator() { }
+  private:
+    virtual ~Mutator() { }
+
+    friend class NullPrincipalURI;
+  };
+
+  friend class BaseURIMutator<NullPrincipalURI>;
 };
 
 #endif // __NullPrincipalURI_h__

devtools/client/framework/toolbox.js

@@ -1755,9 +1755,47 @@ Toolbox.prototype = {
         node.removeAttribute("selected");
         node.removeAttribute("aria-selected");
       }
+      // The webconsole panel is in a special location due to split console
+      if (!node.id) {
+        node = this.webconsolePanel;
+      }
+
+      let iframe = node.querySelector(".toolbox-panel-iframe");
+      if (iframe) {
+        let visible = node.id == id;
+        // Prevents hiding the split-console if it is currently enabled
+        if (node == this.webconsolePanel && this.splitConsole) {
+          visible = true;
+        }
+        this.setIframeVisible(iframe, visible);
+      }
     });
   },
 
+  /**
+   * Make a privileged iframe visible/hidden.
+   *
+   * For now, XUL Iframes loading chrome documents (i.e. <iframe type!="content" />)
+   * can't be hidden at platform level. And so don't support 'visibilitychange' event.
+   *
+   * This helper workarounds that by at least being able to send these kind of events.
+   * It will help panel react differently depending on them being displayed or in
+   * background.
+   */
+  setIframeVisible: function (iframe, visible) {
+    let state = visible ? "visible" : "hidden";
+    let win = iframe.contentWindow;
+    let doc = win.document;
+    if (doc.visibilityState != state) {
+      // 1) Overload document's `visibilityState` attribute
+      // Use defineProperty, as by default `document.visbilityState` is read only.
+      Object.defineProperty(doc, "visibilityState", { value: state, configurable: true });
+
+      // 2) Fake the 'visibilitychange' event
+      win.dispatchEvent(new win.Event("visibilitychange"));
+    }
+  },
+
   /**
    * Switch to the tool with the given id
    *
@@ -1873,6 +1911,12 @@ Toolbox.prototype = {
     Services.prefs.setBoolPref(SPLITCONSOLE_ENABLED_PREF, true);
     this._refreshConsoleDisplay();
 
+    // Ensure split console is visible if console was already loaded in background
+    let iframe = this.webconsolePanel.querySelector(".toolbox-panel-iframe");
+    if (iframe) {
+      this.setIframeVisible(iframe, true);
+    }
+
     return this.loadTool("webconsole").then(() => {
       this.emit("split-console");
       this.focusConsoleInput();
@@ -1989,8 +2033,8 @@ Toolbox.prototype = {
    */
   async _onWillNavigate() {
     let toolId = this.currentToolId;
-    // For now, only inspector and webconsole fires "reloaded" event
-    if (toolId != "inspector" && toolId != "webconsole") {
+    // For now, only inspector, webconsole and netmonitor fire "reloaded" event
+    if (toolId != "inspector" && toolId != "webconsole" && toolId != "netmonitor") {
       return;
     }
 

devtools/client/inspector/inspector.js

@@ -131,7 +131,6 @@ function Inspector(toolbox) {
   this.onSidebarShown = this.onSidebarShown.bind(this);
 
   this._target.on("will-navigate", this._onBeforeNavigate);
-  this._detectingActorFeatures = this._detectActorFeatures();
 }
 
 Inspector.prototype = {
@@ -215,31 +214,6 @@ Inspector.prototype = {
     }
   },
 
-  /**
-   * Figure out what features the backend supports
-   */
-  _detectActorFeatures: function () {
-    this._supportsDuplicateNode = false;
-    this._supportsScrollIntoView = false;
-    this._supportsResolveRelativeURL = false;
-
-    // Use getActorDescription first so that all actorHasMethod calls use
-    // a cached response from the server.
-    return this._target.getActorDescription("domwalker").then(desc => {
-      return promise.all([
-        this._target.actorHasMethod("domwalker", "duplicateNode").then(value => {
-          this._supportsDuplicateNode = value;
-        }).catch(console.error),
-        this._target.actorHasMethod("domnode", "scrollIntoView").then(value => {
-          this._supportsScrollIntoView = value;
-        }).catch(console.error),
-        this._target.actorHasMethod("inspector", "resolveRelativeURL").then(value => {
-          this._supportsResolveRelativeURL = value;
-        }).catch(console.error),
-      ]);
-    });
-  },
-
   _deferredOpen: async function (defaultSelection) {
     this.breadcrumbs = new HTMLBreadcrumbs(this);
 
@@ -864,8 +838,6 @@ Inspector.prototype = {
     try {
       let hasSupportsHighlighters =
         yield this.target.actorHasMethod("inspector", "supportsHighlighters");
-      let hasPickColorFromPage =
-        yield this.target.actorHasMethod("inspector", "pickColorFromPage");
 
       let supportsHighlighters;
       if (hasSupportsHighlighters) {
@@ -877,7 +849,7 @@ Inspector.prototype = {
         supportsHighlighters = nodeFront && nodeFront.isInHTMLDocument;
       }
 
-      return supportsHighlighters && hasPickColorFromPage;
+      return supportsHighlighters;
     } catch (e) {
       console.error(e);
       return false;
@@ -1299,7 +1271,6 @@ Inspector.prototype = {
     menu.append(new MenuItem({
       id: "node-menu-duplicatenode",
       label: INSPECTOR_L10N.getStr("inspectorDuplicateNode.label"),
-      hidden: !this._supportsDuplicateNode,
       disabled: !isDuplicatableElement,
       click: () => this.duplicateNode(),
     }));
@@ -1383,7 +1354,6 @@ Inspector.prototype = {
       label: INSPECTOR_L10N.getStr("inspectorScrollNodeIntoView.label"),
       accesskey:
         INSPECTOR_L10N.getStr("inspectorScrollNodeIntoView.accesskey"),
-      hidden: !this._supportsScrollIntoView,
       disabled: !isSelectionElement,
       click: () => this.scrollNodeIntoView(),
     }));
@@ -1600,8 +1570,7 @@ Inspector.prototype = {
     }
 
     let type = popupNode.dataset.type;
-    if (this._supportsResolveRelativeURL &&
-        (type === "uri" || type === "cssresource" || type === "jsresource")) {
+    if ((type === "uri" || type === "cssresource" || type === "jsresource")) {
       // Links can't be opened in new tabs in the browser toolbox.
       if (type === "uri" && !this.target.chrome) {
         linkFollow.visible = true;
@@ -1732,8 +1701,7 @@ Inspector.prototype = {
    * @return {Promise} resolves when the eyedropper is hidden.
    */
   hideEyeDropper: function () {
-    // The eyedropper button doesn't exist, most probably because the actor doesn't
-    // support the pickColorFromPage, or because the page isn't HTML.
+    // The eyedropper button doesn't exist, most probably  because the page isn't HTML.
     if (!this.eyeDropperButton) {
       return null;
     }
@@ -2127,8 +2095,6 @@ Inspector.prototype = {
 
     if (type === "uri" || type === "cssresource" || type === "jsresource") {
       // Open link in a new tab.
-      // When the inspector menu was setup on click (see _getNodeLinkMenuItems), we
-      // already checked that resolveRelativeURL existed.
       this.inspector.resolveRelativeURL(
         link, this.selection.nodeFront).then(url => {
           if (type === "uri") {
@@ -2169,8 +2135,6 @@ Inspector.prototype = {
    * This method is here for the benefit of copying links.
    */
   copyAttributeLink: function (link) {
-    // When the inspector menu was setup on click (see _getNodeLinkMenuItems), we
-    // already checked that resolveRelativeURL existed.
     this.inspector.resolveRelativeURL(link, this.selection.nodeFront).then(url => {
       clipboardHelper.copyString(url);
     }, console.error);

devtools/client/inspector/markup/test/browser_markup_links_04.js

@@ -93,12 +93,6 @@ add_task(function* () {
     let linkFollow = allMenuItems.find(i => i.id === "node-menu-link-follow");
     let linkCopy = allMenuItems.find(i => i.id === "node-menu-link-copy");
 
-    // The contextual menu setup is async, because it needs to know if the
-    // inspector has the resolveRelativeURL method first. So call actorHasMethod
-    // here too to make sure the first call resolves first and the menu is
-    // properly setup.
-    yield inspector.target.actorHasMethod("inspector", "resolveRelativeURL");
-
     is(linkFollow.visible, test.isLinkFollowItemVisible,
       "The follow-link item display is correct");
     is(linkCopy.visible, test.isLinkCopyItemVisible,

devtools/client/inspector/test/shared-head.js

@@ -30,9 +30,6 @@ var openInspector = Task.async(function* (hostType) {
     yield inspector.once("inspector-updated");
   }
 
-  info("Waiting for actor features to be detected");
-  yield inspector._detectingActorFeatures;
-
   yield registerTestActor(toolbox.target.client);
   let testActor = yield getTestActor(toolbox);
 

devtools/client/jsonview/converter-observer.js

@@ -107,7 +107,7 @@ const JsonSnifferFactory = {
     if (outer) {
       throw Cr.NS_ERROR_NO_AGGREGATION;
     }
-    return new JsonViewSniffer();
+    return new JsonViewSniffer().QueryInterface(iid);
   }
 };
 
@@ -121,7 +121,7 @@ const JsonViewFactory = {
     if (outer) {
       throw Cr.NS_ERROR_NO_AGGREGATION;
     }
-    return JsonViewService.createInstance();
+    return JsonViewService.createInstance().QueryInterface(iid);
   }
 };
 

devtools/client/netmonitor/.eslintrc.js

@@ -20,4 +20,10 @@ module.exports = {
     /* eslint-disable max-len */
     "mozilla/reject-some-requires": ["error", "^(chrome|chrome:.*|resource:.*|devtools/server/.*|.*\\.jsm|devtools/shared/platform/(chome|content)/.*)$"],
   },
+
+  "parserOptions": {
+    "ecmaFeatures": {
+      experimentalObjectRestSpread: true,
+    },
+  },
 };

devtools/client/netmonitor/index.html

@@ -45,7 +45,7 @@
       window.connector = connector;
 
       window.Netmonitor = {
-        bootstrap({ toolbox }) {
+        bootstrap({ toolbox, panel }) {
           this.mount = document.querySelector("#mount");
 
           const connection = {
@@ -53,6 +53,7 @@
               tabTarget: toolbox.target,
             },
             toolbox,
+            panel,
           };
 
           const openLink = (link) => {

devtools/client/netmonitor/panel.js

@@ -16,6 +16,7 @@ NetMonitorPanel.prototype = {
     }
     await this.panelWin.Netmonitor.bootstrap({
       toolbox: this.toolbox,
+      panel: this,
     });
     this.emit("ready");
     this.isReady = true;

devtools/client/netmonitor/src/components/RequestListContent.js

@@ -37,7 +37,7 @@ class RequestListContent extends Component {
       connector: PropTypes.object.isRequired,
       columns: PropTypes.object.isRequired,
       dispatch: PropTypes.func.isRequired,
-      displayedRequests: PropTypes.object.isRequired,
+      displayedRequests: PropTypes.array.isRequired,
       firstRequestStartedMillis: PropTypes.number.isRequired,
       fromCache: PropTypes.bool,
       onCauseBadgeMouseDown: PropTypes.func.isRequired,

devtools/client/netmonitor/src/components/StatisticsPanel.js

@@ -40,7 +40,7 @@ class StatisticsPanel extends Component {
       connector: PropTypes.object.isRequired,
       closeStatistics: PropTypes.func.isRequired,
       enableRequestFilterTypeOnly: PropTypes.func.isRequired,
-      requests: PropTypes.object,
+      requests: PropTypes.array,
     };
   }
 
@@ -67,7 +67,7 @@ class StatisticsPanel extends Component {
     MediaQueryList.addListener(this.onLayoutChange);
 
     const { requests } = this.props;
-    let ready = requests && !requests.isEmpty() && requests.every((req) =>
+    let ready = requests && requests.length && requests.every((req) =>
       req.contentSize !== undefined && req.mimeType && req.responseHeaders &&
       req.status !== undefined && req.totalTime !== undefined
     );

devtools/client/netmonitor/src/components/Toolbar.js

@@ -71,7 +71,7 @@ class Toolbar extends Component {
       toggleBrowserCache: PropTypes.func.isRequired,
       browserCacheDisabled: PropTypes.bool.isRequired,
       toggleRequestFilterType: PropTypes.func.isRequired,
-      filteredRequests: PropTypes.object.isRequired,
+      filteredRequests: PropTypes.array.isRequired,
     };
   }
 

devtools/client/netmonitor/src/connector/firefox-connector.js

@@ -15,6 +15,7 @@ class FirefoxConnector {
     this.connect = this.connect.bind(this);
     this.disconnect = this.disconnect.bind(this);
     this.willNavigate = this.willNavigate.bind(this);
+    this.navigate = this.navigate.bind(this);
     this.displayCachedEvents = this.displayCachedEvents.bind(this);
     this.onDocLoadingMarker = this.onDocLoadingMarker.bind(this);
     this.sendHTTPRequest = this.sendHTTPRequest.bind(this);
@@ -34,6 +35,7 @@ class FirefoxConnector {
     this.getState = getState;
     this.tabTarget = connection.tabConnection.tabTarget;
     this.toolbox = connection.toolbox;
+    this.panel = connection.panel;
 
     this.webConsoleClient = this.tabTarget.activeConsole;
 
@@ -50,6 +52,7 @@ class FirefoxConnector {
     // Paused network panel should be automatically resumed when page
     // reload, so `will-navigate` listener needs to be there all the time.
     this.tabTarget.on("will-navigate", this.willNavigate);
+    this.tabTarget.on("navigate", this.navigate);
 
     // Don't start up waiting for timeline markers if the server isn't
     // recent enough to emit the markers we're interested in.
@@ -120,6 +123,25 @@ class FirefoxConnector {
     }
   }
 
+  navigate() {
+    if (this.dataProvider.isPayloadQueueEmpty()) {
+      this.onReloaded();
+      return;
+    }
+    let listener = () => {
+      if (!this.dataProvider.isPayloadQueueEmpty()) {
+        return;
+      }
+      window.off(EVENTS.PAYLOAD_READY, listener);
+      this.onReloaded();
+    };
+    window.on(EVENTS.PAYLOAD_READY, listener);
+  }
+
+  onReloaded() {
+    this.panel.emit("reloaded");
+  }
+
   /**
    * Display any network events already in the cache.
    */

devtools/client/netmonitor/src/connector/firefox-data-provider.js

@@ -231,6 +231,15 @@ class FirefoxDataProvider {
     return this.payloadQueue.find((item) => item.id === id);
   }
 
+  /**
+   * Public API used by the Toolbox: Tells if there is still any pending request.
+   *
+   * @return {boolean} returns true if the payload queue is empty
+   */
+  isPayloadQueueEmpty() {
+    return this.payloadQueue.length === 0;
+  }
+
   /**
    * Return true if payload is ready (all data fetched from the backend)
    *

devtools/client/netmonitor/src/reducers/requests.js

@@ -4,7 +4,6 @@
 
 "use strict";
 
-const I = require("devtools/client/shared/vendor/immutable");
 const {
   getUrlDetails,
   processNetworkUpdates,
@@ -21,59 +20,173 @@ const {
   UPDATE_REQUEST,
 } = require("../constants");
 
-const Request = I.Record({
-  id: null,
-  // Set to true in case of a request that's being edited as part of "edit and resend"
-  isCustom: false,
-  // Request properties - at the beginning, they are unknown and are gradually filled in
-  startedMillis: undefined,
-  endedMillis: undefined,
-  method: undefined,
-  url: undefined,
-  urlDetails: undefined,
-  remotePort: undefined,
-  remoteAddress: undefined,
-  isXHR: undefined,
-  cause: undefined,
-  fromCache: undefined,
-  fromServiceWorker: undefined,
-  status: undefined,
-  statusText: undefined,
-  httpVersion: undefined,
-  securityState: undefined,
-  securityInfo: undefined,
-  mimeType: "text/plain",
-  contentSize: undefined,
-  transferredSize: undefined,
-  totalTime: undefined,
-  eventTimings: undefined,
-  headersSize: undefined,
-  // Text value is used for storing custom request query
-  // which only appears when user edit the custom requst form
-  customQueryValue: undefined,
-  requestHeaders: undefined,
-  requestHeadersFromUploadStream: undefined,
-  requestCookies: undefined,
-  requestPostData: undefined,
-  responseHeaders: undefined,
-  responseCookies: undefined,
-  responseContent: undefined,
-  responseContentAvailable: false,
-  formDataSections: undefined,
-});
+/**
+ * This structure stores list of all HTTP requests received
+ * from the backend. It's using plain JS structures to store
+ * data instead of ImmutableJS, which is performance expensive.
+ */
+function Requests() {
+  return {
+    // Map with all requests (key = actor ID, value = request object)
+    requests: mapNew(),
+    // Selected request ID
+    selectedId: null,
+    preselectedId: null,
+    // True if the monitor is recording HTTP traffic
+    recording: true,
+    // Auxiliary fields to hold requests stats
+    firstStartedMillis: +Infinity,
+    lastEndedMillis: -Infinity,
+  };
+}
 
-const Requests = I.Record({
-  // The collection of requests (keyed by id)
-  requests: I.Map(),
-  // Selection state
-  selectedId: null,
-  preselectedId: null,
-  // Auxiliary fields to hold requests stats
-  firstStartedMillis: +Infinity,
-  lastEndedMillis: -Infinity,
-  // Recording state
-  recording: true,
-});
+/**
+ * This reducer is responsible for maintaining list of request
+ * within the Network panel.
+ */
+function requestsReducer(state = Requests(), action) {
+  switch (action.type) {
+    // Appending new request into the list/map.
+    case ADD_REQUEST: {
+      let nextState = { ...state };
+
+      let newRequest = {
+        id: action.id,
+        ...action.data,
+        urlDetails: getUrlDetails(action.data.url),
+      };
+
+      nextState.requests = mapSet(state.requests, newRequest.id, newRequest);
+
+      // Update the started/ended timestamps.
+      let { startedMillis } = action.data;
+      if (startedMillis < state.firstStartedMillis) {
+        nextState.firstStartedMillis = startedMillis;
+      }
+      if (startedMillis > state.lastEndedMillis) {
+        nextState.lastEndedMillis = startedMillis;
+      }
+
+      // Select the request if it was preselected and there is no other selection.
+      if (state.preselectedId && state.preselectedId === action.id) {
+        nextState.selectedId = state.selectedId || state.preselectedId;
+        nextState.preselectedId = null;
+      }
+
+      return nextState;
+    }
+
+    // Update an existing request (with received data).
+    case UPDATE_REQUEST: {
+      let { requests, lastEndedMillis } = state;
+
+      let request = requests.get(action.id);
+      if (!request) {
+        return state;
+      }
+
+      request = {
+        ...request,
+        ...processNetworkUpdates(action.data),
+      };
+
+      return {
+        ...state,
+        requests: mapSet(state.requests, action.id, request),
+        lastEndedMillis: lastEndedMillis,
+      };
+    }
+
+    // Remove all requests in the list. Create fresh new state
+    // object, but keep value of the `recording` field.
+    case CLEAR_REQUESTS: {
+      return {
+        ...Requests(),
+        recording: state.recording,
+      };
+    }
+
+    // Select specific request.
+    case SELECT_REQUEST: {
+      return {
+        ...state,
+        selectedId: action.id,
+      };
+    }
+
+    // Clone selected request for re-send.
+    case CLONE_SELECTED_REQUEST: {
+      let { requests, selectedId } = state;
+
+      if (!selectedId) {
+        return state;
+      }
+
+      let clonedRequest = requests.get(selectedId);
+      if (!clonedRequest) {
+        return state;
+      }
+
+      let newRequest = {
+        id: clonedRequest.id + "-clone",
+        method: clonedRequest.method,
+        url: clonedRequest.url,
+        urlDetails: clonedRequest.urlDetails,
+        requestHeaders: clonedRequest.requestHeaders,
+        requestPostData: clonedRequest.requestPostData,
+        isCustom: true
+      };
+
+      return {
+        ...state,
+        requests: mapSet(requests, newRequest.id, newRequest),
+        selectedId: newRequest.id,
+      };
+    }
+
+    // Removing temporary cloned request (created for re-send, but canceled).
+    case REMOVE_SELECTED_CUSTOM_REQUEST: {
+      return closeCustomRequest(state);
+    }
+
+    // Re-sending an existing request.
+    case SEND_CUSTOM_REQUEST: {
+      // When a new request with a given id is added in future, select it immediately.
+      // where we know in advance the ID of the request, at a time when it
+      // wasn't sent yet.
+      return closeCustomRequest(state.set("preselectedId", action.id));
+    }
+
+    // Pause/resume button clicked.
+    case TOGGLE_RECORDING: {
+      return {
+        ...state,
+        recording: !state.recording,
+      };
+    }
+
+    // Side bar with request details opened.
+    case OPEN_NETWORK_DETAILS: {
+      let nextState = { ...state };
+      if (!action.open) {
+        nextState.selectedId = null;
+        return nextState;
+      }
+
+      if (!state.selectedId && !state.requests.isEmpty()) {
+        nextState.selectedId = [...state.requests.values()][0].id;
+        return nextState;
+      }
+
+      return state;
+    }
+
+    default:
+      return state;
+  }
+}
+
+// Helpers
 
 /**
  * Remove the currently selected custom request.
@@ -92,119 +205,41 @@ function closeCustomRequest(state) {
     return state;
   }
 
-  return state.withMutations(st => {
-    st.requests = st.requests.delete(selectedId);
-    st.selectedId = null;
-  });
+  return {
+    ...state,
+    requests: mapDelete(state.requests, selectedId),
+    selectedId: null,
+  };
 }
 
-function requestsReducer(state = new Requests(), action) {
-  switch (action.type) {
-    case ADD_REQUEST: {
-      return state.withMutations(st => {
-        let newRequest = new Request(Object.assign(
-          { id: action.id },
-          action.data,
-          { urlDetails: getUrlDetails(action.data.url) }
-        ));
-        st.requests = st.requests.set(newRequest.id, newRequest);
+// Immutability helpers
+// FIXME The following helper API need refactoring, see bug 1418969.
 
-        // Update the started/ended timestamps
-        let { startedMillis } = action.data;
-        if (startedMillis < st.firstStartedMillis) {
-          st.firstStartedMillis = startedMillis;
-        }
-        if (startedMillis > st.lastEndedMillis) {
-          st.lastEndedMillis = startedMillis;
-        }
+/**
+ * Clone an existing map.
+ */
+function mapNew(map) {
+  let newMap = new Map(map);
+  newMap.isEmpty = () => newMap.size == 0;
+  newMap.valueSeq = () => [...newMap.values()];
+  return newMap;
+}
 
-        // Select the request if it was preselected and there is no other selection
-        if (st.preselectedId && st.preselectedId === action.id) {
-          st.selectedId = st.selectedId || st.preselectedId;
-          st.preselectedId = null;
-        }
-      });
-    }
-    case CLEAR_REQUESTS: {
-      return new Requests({
-        recording: state.recording
-      });
-    }
-    case CLONE_SELECTED_REQUEST: {
-      let { requests, selectedId } = state;
+/**
+ * Append new item into existing map and return new map.
+ */
+function mapSet(map, key, value) {
+  let newMap = mapNew(map);
+  return newMap.set(key, value);
+}
 
-      if (!selectedId) {
-        return state;
-      }
-
-      let clonedRequest = requests.get(selectedId);
-      if (!clonedRequest) {
-        return state;
-      }
-
-      let newRequest = new Request({
-        id: clonedRequest.id + "-clone",
-        method: clonedRequest.method,
-        url: clonedRequest.url,
-        urlDetails: clonedRequest.urlDetails,
-        requestHeaders: clonedRequest.requestHeaders,
-        requestPostData: clonedRequest.requestPostData,
-        isCustom: true
-      });
-
-      return state.withMutations(st => {
-        st.requests = requests.set(newRequest.id, newRequest);
-        st.selectedId = newRequest.id;
-      });
-    }
-    case OPEN_NETWORK_DETAILS: {
-      if (!action.open) {
-        return state.set("selectedId", null);
-      }
-
-      if (!state.selectedId && !state.requests.isEmpty()) {
-        return state.set("selectedId", state.requests.first().id);
-      }
-
-      return state;
-    }
-    case REMOVE_SELECTED_CUSTOM_REQUEST: {
-      return closeCustomRequest(state);
-    }
-    case SELECT_REQUEST: {
-      return state.set("selectedId", action.id);
-    }
-    case SEND_CUSTOM_REQUEST: {
-      // When a new request with a given id is added in future, select it immediately.
-      // where we know in advance the ID of the request, at a time when it
-      // wasn't sent yet.
-      return closeCustomRequest(state.set("preselectedId", action.id));
-    }
-    case TOGGLE_RECORDING: {
-      return state.set("recording", !state.recording);
-    }
-    case UPDATE_REQUEST: {
-      let { requests, lastEndedMillis } = state;
-
-      let updatedRequest = requests.get(action.id);
-      if (!updatedRequest) {
-        return state;
-      }
-
-      updatedRequest = updatedRequest.withMutations(request => {
-        let values = processNetworkUpdates(action.data);
-        request = Object.assign(request, values);
-      });
-
-      return state.withMutations(st => {
-        st.requests = requests.set(updatedRequest.id, updatedRequest);
-        st.lastEndedMillis = lastEndedMillis;
-      });
-    }
-
-    default:
-      return state;
-  }
+/**
+ * Remove an item from existing map and return new map.
+ */
+function mapDelete(map, key) {
+  let newMap = mapNew(map);
+  newMap.requests.delete(key);
+  return newMap;
 }
 
 module.exports = {

devtools/client/netmonitor/src/selectors/requests.js

@@ -56,9 +56,9 @@ const getTypeFilterFn = createSelector(
 );
 
 const getSortFn = createSelector(
-  state => state.requests.requests,
+  state => state.requests,
   state => state.sort,
-  (requests, sort) => {
+  ({ requests }, sort) => {
     const sorter = Sorters[sort.type || "waterfall"];
     const ascending = sort.ascending ? +1 : -1;
     return (a, b) => ascending * sortWithClones(requests, sorter, a, b);
@@ -66,23 +66,34 @@ const getSortFn = createSelector(
 );
 
 const getSortedRequests = createSelector(
-  state => state.requests.requests,
+  state => state.requests,
   getSortFn,
-  (requests, sortFn) => requests.valueSeq().sort(sortFn).toList()
+  ({ requests }, sortFn) => {
+    let arr = requests.valueSeq().sort(sortFn);
+    arr.get = index => arr[index];
+    arr.isEmpty = () => this.length == 0;
+    arr.size = arr.length;
+    return arr;
+  }
 );
 
 const getDisplayedRequests = createSelector(
-  state => state.requests.requests,
+  state => state.requests,
   getFilterFn,
   getSortFn,
-  (requests, filterFn, sortFn) => requests.valueSeq()
-    .filter(filterFn).sort(sortFn).toList()
+  ({ requests }, filterFn, sortFn) => {
+    let arr = requests.valueSeq().filter(filterFn).sort(sortFn);
+    arr.get = index => arr[index];
+    arr.isEmpty = () => this.length == 0;
+    arr.size = arr.length;
+    return arr;
+  }
 );
 
 const getTypeFilteredRequests = createSelector(
-  state => state.requests.requests,
+  state => state.requests,
   getTypeFilterFn,
-  (requests, filterFn) => requests.valueSeq().filter(filterFn).toList()
+  ({ requests }, filterFn) => requests.valueSeq().filter(filterFn)
 );
 
 const getDisplayedRequestsSummary = createSelector(

devtools/client/netmonitor/src/selectors/timing-markers.js

@@ -5,7 +5,7 @@
 "use strict";
 
 function getDisplayedTimingMarker(state, marker) {
-  return state.timingMarkers.get(marker) - state.requests.get("firstStartedMillis");
+  return state.timingMarkers.get(marker) - state.requests.firstStartedMillis;
 }
 
 module.exports = {

devtools/client/netmonitor/src/selectors/ui.js

@@ -8,7 +8,7 @@ const { REQUESTS_WATERFALL } = require("../constants");
 const { getDisplayedRequests } = require("./requests");
 
 function isNetworkDetailsToggleButtonDisabled(state) {
-  return getDisplayedRequests(state).isEmpty();
+  return getDisplayedRequests(state).length == 0;
 }
 
 const EPSILON = 0.001;

devtools/client/netmonitor/test/browser_net_copy_params.js

@@ -98,8 +98,8 @@ add_task(function* () {
     // since copyPostData API needs to read these state.
     yield waitUntil(() => {
       let { requests } = store.getState().requests;
-      let actIDs = Object.keys(requests.toJS());
-      let { formDataSections, requestPostData } = requests.get(actIDs[index]).toJS();
+      let actIDs = [...requests.keys()];
+      let { formDataSections, requestPostData } = requests.get(actIDs[index]);
       return formDataSections && requestPostData;
     });
     EventUtils.sendMouseEvent({ type: "mousedown" },

devtools/client/shared/components/VisibilityHandler.js

@@ -0,0 +1,47 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+/**
+ * Helper class to disable panel rendering when it is in background.
+ *
+ * Toolbox code hides the iframes when switching to another panel
+ * and triggers `visibilitychange` events.
+ *
+ * See devtools/client/framework/toolbox.js:setIframeVisible().
+ */
+
+const {
+  createClass,
+} = require("devtools/client/shared/vendor/react");
+
+const VisibilityHandler = createClass({
+
+  displayName: "VisiblityHandler",
+
+  shouldComponentUpdate() {
+    return document.visibilityState == "visible";
+  },
+
+  onVisibilityChange() {
+    if (document.visibilityState == "visible") {
+      this.forceUpdate();
+    }
+  },
+
+  componentDidMount() {
+    window.addEventListener("visibilitychange", this.onVisibilityChange);
+  },
+
+  componentWillUnmount() {
+    window.removeEventListener("visibilitychange", this.onVisibilityChange);
+  },
+
+  render() {
+    return this.props.children;
+  }
+});
+
+module.exports = VisibilityHandler;

devtools/client/shared/components/moz.build

@@ -22,6 +22,7 @@ DevToolsModules(
     'SidebarToggle.js',
     'StackTrace.js',
     'Tree.js',
+    'VisibilityHandler.js',
 )
 
 MOCHITEST_CHROME_MANIFESTS += ['test/mochitest/chrome.ini']

devtools/client/webconsole/new-console-output/components/ConsoleOutput.js

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
-const { Component, createFactory } = require("devtools/client/shared/vendor/react");
+const { Component, createElement, createFactory } = require("devtools/client/shared/vendor/react");
 const dom = require("devtools/client/shared/vendor/react-dom-factories");
 const PropTypes = require("devtools/client/shared/vendor/react-prop-types");
 const { connect } = require("devtools/client/shared/vendor/react-redux");
@@ -18,6 +18,7 @@ const {
   getAllRepeatById,
 } = require("devtools/client/webconsole/new-console-output/selectors/messages");
 const MessageContainer = createFactory(require("devtools/client/webconsole/new-console-output/components/MessageContainer").MessageContainer);
+const VisibilityHandler = createFactory(require("devtools/client/shared/components/VisibilityHandler"));
 const {
   MESSAGE_TYPE,
 } = require("devtools/client/webconsole/new-console-output/constants");
@@ -190,4 +191,9 @@ function mapStateToProps(state, props) {
   };
 }
 
-module.exports = connect(mapStateToProps)(ConsoleOutput);
+module.exports = connect(mapStateToProps)(props =>
+  VisibilityHandler(
+    null,
+    createElement(ConsoleOutput, props)
+  )
+);

devtools/client/webconsole/new-console-output/new-console-output-wrapper.js

@@ -209,7 +209,8 @@ NewConsoleOutputWrapper.prototype = {
     // be removed once it's not needed anymore.
     // Can only wait for response if the action contains a valid message.
     let promise;
-    if (waitForResponse) {
+    // Also, do not expect any update while the panel is in background.
+    if (waitForResponse && document.visibilityState === "visible") {
       promise = new Promise(resolve => {
         let jsterm = this.jsterm;
         jsterm.hud.on("new-messages", function onThisMessage(e, messages) {

devtools/client/webconsole/new-console-output/test/mochitest/browser.ini

@@ -440,6 +440,7 @@ skip-if = true #	Bug 1403205
 [browser_webconsole_violation.js]
 skip-if = true #	Bug 1405245
 # old console skip-if = e10s && (os == 'win') # Bug 1264955
+[browser_webconsole_visibility_messages.js]
 [browser_webconsole_warn_about_replaced_api.js]
 [browser_webconsole_websocket.js]
-skip-if = true # Bug 1408950
+skip-if = true # Bug 1408950

devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_timestamps.js

@@ -35,6 +35,9 @@ add_task(async function() {
   await togglePref(optionsPanel, observer);
   observer.destroy();
 
+  // Switch back to the console as it won't update when it is in background
+  await toolbox.selectTool("webconsole");
+
   await testChangedPref(hud);
 
   Services.prefs.clearUserPref(PREF_MESSAGE_TIMESTAMP);

devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_visibility_messages.js

@@ -0,0 +1,116 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+// Check messages logged when console not visible are displayed when
+// the user show the console again.
+
+const HTML = `
+  <!DOCTYPE html>
+  <html>
+    <body>
+      <h1>Test console visibility update</h1>
+      <script>
+        function log(str) {
+          console.log(str);
+        }
+      </script>
+    </body>
+  </html>
+`;
+const TEST_URI = "data:text/html;charset=utf-8," + encodeURI(HTML);
+const MESSAGES_COUNT = 10;
+
+add_task(async function () {
+  const hud = await openNewTabAndConsole(TEST_URI);
+  const toolbox = gDevTools.getToolbox(hud.target);
+
+  info("Log one message in the console");
+  ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    content.wrappedJSObject.log("in-console log");
+  });
+  await waitFor(() => findMessage(hud, "in-console log"));
+
+  info("select the inspector");
+  await toolbox.selectTool("inspector");
+
+  info("Wait for console to be hidden");
+  const { document } = hud.iframeWindow;
+  await waitFor(() => (document.visibilityState == "hidden"));
+
+  const onAllMessagesInStore = new Promise(done => {
+    const store = hud.ui.newConsoleOutput.getStore();
+    store.subscribe(() => {
+      const messages = store.getState().messages.messagesById.size;
+      // Also consider the "in-console log" message
+      if (messages == MESSAGES_COUNT+1) {
+        done();
+      }
+    });
+  });
+
+  await ContentTask.spawn(gBrowser.selectedBrowser, [MESSAGES_COUNT], (count) => {
+    for (let i = 1; i <= count; i++) {
+      content.wrappedJSObject.log("in-inspector log " + i);
+    }
+  });
+
+  info("Waiting for all messages to be logged into the store");
+  await onAllMessagesInStore;
+
+  const count = await findMessages(hud, "in-inspector");
+  is(count, 0, "No messages from the inspector actually appear in the console");
+
+  info("select back the console");
+  await toolbox.selectTool("webconsole");
+
+  info("And wait for all messages to be visible");
+  let waitForMessagePromises = [];
+  for (let j = 1; j <= MESSAGES_COUNT; j++) {
+    waitForMessagePromises.push(waitFor(() => findMessage(hud, "in-inspector log " + j)));
+  }
+
+  await Promise.all(waitForMessagePromises);
+  ok(true, "All the messages logged when the console was hidden were displayed.");
+});
+
+// Similar scenario, but with the split console on the inspector panel.
+// Here, the messages should still be logged.
+add_task(async function () {
+  const hud = await openNewTabAndConsole(TEST_URI);
+  const toolbox = gDevTools.getToolbox(hud.target);
+
+  info("Log one message in the console");
+  ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    content.wrappedJSObject.log("in-console log");
+  });
+  await waitFor(() => findMessage(hud, "in-console log"));
+
+  info("select the inspector");
+  await toolbox.selectTool("inspector");
+
+  info("Wait for console to be hidden");
+  const { document } = hud.iframeWindow;
+  await waitFor(() => (document.visibilityState == "hidden"));
+
+  await toolbox.openSplitConsole();
+
+  await ContentTask.spawn(gBrowser.selectedBrowser, [MESSAGES_COUNT], (count) => {
+    for (let i = 1; i <= count; i++) {
+      content.wrappedJSObject.log("in-inspector log " + i);
+    }
+  });
+
+  info("Wait for all messages to be visible in the split console");
+  let waitForMessagePromises = [];
+  for (let j = 1; j <= MESSAGES_COUNT; j++) {
+    waitForMessagePromises.push(waitFor(() => findMessage(hud, "in-inspector log " + j)));
+  }
+
+  await Promise.all(waitForMessagePromises);
+  ok(true, "All the messages logged when we are using the split console");
+
+  await toolbox.closeSplitConsole();
+});

devtools/server/actors/animation.js

@@ -509,7 +509,10 @@ var AnimationPlayerActor = protocol.ActorClassWithSpec(animationPlayerSpec, {
             target = target.parentElement;
           }
           const value =
-            DOMWindowUtils.getUnanimatedComputedStyle(target, pseudo, property.name);
+            DOMWindowUtils.getUnanimatedComputedStyle(target,
+                                                      pseudo,
+                                                      property.name,
+                                                      DOMWindowUtils.FLUSH_NONE);
           const animationType = DOMWindowUtils.getAnimationTypeForLonghand(property.name);
           underlyingValue = animationType === "float" ? parseFloat(value, 10) : value;
         }

devtools/shared/webconsole/network-monitor.js

@@ -439,13 +439,34 @@ NetworkResponseListener.prototype = {
     // we pass the data from our pipe to the converter.
     this.offset = 0;
 
+    let channel = this.request;
+
+    // Bug 1372115 - We should load bytecode cached requests from cache as the actual
+    // channel content is going to be optimized data that reflects platform internals
+    // instead of the content user expects (i.e. content served by HTTP server)
+    // Note that bytecode cached is one example, there may be wasm or other usecase in
+    // future.
+    let isOptimizedContent = false;
+    try {
+      if (channel instanceof Ci.nsICacheInfoChannel) {
+        isOptimizedContent = channel.alternativeDataType;
+      }
+    } catch (e) {
+      // Accessing `alternativeDataType` for some SW requests throws.
+    }
+    if (isOptimizedContent) {
+      let charset = this.request.contentCharset || this.httpActivity.charset;
+      NetworkHelper.loadFromCache(this.httpActivity.url, charset,
+                                  this._onComplete.bind(this));
+      return;
+    }
+
     // In the multi-process mode, the conversion happens on the child
     // side while we can only monitor the channel on the parent
     // side. If the content is gzipped, we have to unzip it
     // ourself. For that we use the stream converter services.  Do not
     // do that for Service workers as they are run in the child
     // process.
-    let channel = this.request;
     if (!this.httpActivity.fromServiceWorker &&
         channel instanceof Ci.nsIEncodedChannel &&
         channel.contentEncodings &&

dom/base/CustomElementRegistry.h

@@ -480,15 +480,24 @@ public:
 
 class MOZ_RAII AutoCEReaction final {
   public:
-    explicit AutoCEReaction(CustomElementReactionsStack* aReactionsStack)
-      : mReactionsStack(aReactionsStack) {
+    // JSContext is allowed to be a nullptr if we are guaranteeing that we're
+    // not doing something that might throw but not finish reporting a JS
+    // exception during the lifetime of the AutoCEReaction.
+    AutoCEReaction(CustomElementReactionsStack* aReactionsStack, JSContext* aCx)
+      : mReactionsStack(aReactionsStack)
+      , mCx(aCx) {
       mReactionsStack->CreateAndPushElementQueue();
     }
     ~AutoCEReaction() {
+      Maybe<JS::AutoSaveExceptionState> ases;
+      if (mCx) {
+        ases.emplace(mCx);
+      }
       mReactionsStack->PopAndInvokeElementQueue();
     }
   private:
     RefPtr<CustomElementReactionsStack> mReactionsStack;
+    JSContext* mCx;
 };
 
 } // namespace dom

dom/base/FragmentOrElement.cpp

@@ -1214,15 +1214,12 @@ FragmentOrElement::GetBindingParent() const
 }
 
 nsXBLBinding*
-FragmentOrElement::GetXBLBinding() const
+FragmentOrElement::DoGetXBLBinding() const
 {
-  if (HasFlag(NODE_MAY_BE_IN_BINDING_MNGR)) {
-    nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots();
-    if (slots) {
-      return slots->mXBLBinding;
-    }
+  MOZ_ASSERT(HasFlag(NODE_MAY_BE_IN_BINDING_MNGR));
+  if (nsExtendedDOMSlots* slots = GetExistingExtendedDOMSlots()) {
+    return slots->mXBLBinding;
   }
-
   return nullptr;
 }
 

dom/base/FragmentOrElement.h

@@ -147,8 +147,8 @@ public:
   virtual void AppendTextTo(nsAString& aResult) override;
   MOZ_MUST_USE
   virtual bool AppendTextTo(nsAString& aResult, const mozilla::fallible_t&) override;
-  virtual nsIContent *GetBindingParent() const override;
-  virtual nsXBLBinding *GetXBLBinding() const override;
+  virtual nsIContent* GetBindingParent() const override;
+  virtual nsXBLBinding* DoGetXBLBinding() const override;
   virtual void SetXBLBinding(nsXBLBinding* aBinding,
                              nsBindingManager* aOldBindingManager = nullptr) override;
   virtual ShadowRoot *GetContainingShadow() const override;

dom/base/nsCopySupport.cpp

@@ -618,7 +618,7 @@ static nsresult AppendImagePromise(nsITransferable* aTransferable,
   NS_ENSURE_TRUE(mimeService, NS_OK);
 
   nsCOMPtr<nsIURI> imgUri;
-  rv = aImgRequest->GetCurrentURI(getter_AddRefs(imgUri));
+  rv = aImgRequest->GetFinalURI(getter_AddRefs(imgUri));
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsIURL> imgUrl = do_QueryInterface(imgUri);

dom/base/nsDOMWindowUtils.cpp

@@ -2970,6 +2970,7 @@ NS_IMETHODIMP
 nsDOMWindowUtils::GetUnanimatedComputedStyle(nsIDOMElement* aElement,
                                              const nsAString& aPseudoElement,
                                              const nsAString& aProperty,
+                                             int32_t aFlushType,
                                              nsAString& aResult)
 {
   nsCOMPtr<Element> element = do_QueryInterface(aElement);
@@ -2984,6 +2985,20 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(nsIDOMElement* aElement,
     return NS_ERROR_INVALID_ARG;
   }
 
+  switch (aFlushType) {
+    case FLUSH_NONE:
+      break;
+    case FLUSH_STYLE: {
+      nsIDocument* doc = element->GetComposedDoc();
+      if (doc) {
+        doc->FlushPendingNotifications(FlushType::Style);
+      }
+      break;
+    }
+    default:
+      return NS_ERROR_INVALID_ARG;
+  }
+
   nsIPresShell* shell = GetPresShell();
   if (!shell) {
     return NS_ERROR_FAILURE;

dom/base/nsDocument.cpp

@@ -6510,7 +6510,8 @@ nsDocument::RegisterElement(JSContext* aCx, const nsAString& aType,
     return;
   }
 
-  AutoCEReaction ceReaction(this->GetDocGroup()->CustomElementReactionsStack());
+  AutoCEReaction ceReaction(this->GetDocGroup()->CustomElementReactionsStack(),
+                            aCx);
   // Unconditionally convert TYPE to lowercase.
   nsAutoString lcType;
   nsContentUtils::ASCIIToLower(aType, lcType);

dom/base/nsGenericDOMDataNode.cpp

@@ -755,7 +755,7 @@ nsGenericDOMDataNode::SetAssignedSlot(HTMLSlotElement* aSlot)
 }
 
 nsXBLBinding *
-nsGenericDOMDataNode::GetXBLBinding() const
+nsGenericDOMDataNode::DoGetXBLBinding() const
 {
   return nullptr;
 }

dom/base/nsGenericDOMDataNode.h

@@ -168,8 +168,8 @@ public:
   virtual void DumpContent(FILE* out, int32_t aIndent, bool aDumpAll) const override;
 #endif
 
-  virtual nsIContent *GetBindingParent() const override;
-  virtual nsXBLBinding *GetXBLBinding() const override;
+  virtual nsIContent* GetBindingParent() const override;
+  virtual nsXBLBinding* DoGetXBLBinding() const override;
   virtual void SetXBLBinding(nsXBLBinding* aBinding,
                              nsBindingManager* aOldBindingManager = nullptr) override;
   virtual mozilla::dom::ShadowRoot *GetContainingShadow() const override;

dom/base/nsIContent.h

@@ -681,14 +681,23 @@ public:
    *
    * @return the binding parent
    */
-  virtual nsIContent *GetBindingParent() const = 0;
+  virtual nsIContent* GetBindingParent() const = 0;
 
   /**
    * Gets the current XBL binding that is bound to this element.
    *
    * @return the current binding.
    */
-  virtual nsXBLBinding *GetXBLBinding() const = 0;
+  nsXBLBinding* GetXBLBinding() const
+  {
+    if (!HasFlag(NODE_MAY_BE_IN_BINDING_MNGR)) {
+      return nullptr;
+    }
+
+    return DoGetXBLBinding();
+  }
+
+  virtual nsXBLBinding* DoGetXBLBinding() const = 0;
 
   /**
    * Sets or unsets an XBL binding for this element. Setting a

dom/base/nsImageLoadingContent.cpp

@@ -757,6 +757,17 @@ nsImageLoadingContent::GetCurrentURI(nsIURI** aURI)
   return result.StealNSResult();
 }
 
+already_AddRefed<nsIURI>
+nsImageLoadingContent::GetCurrentRequestFinalURI()
+{
+  nsCOMPtr<nsIURI> uri;
+  if (mCurrentRequest) {
+    mCurrentRequest->GetFinalURI(getter_AddRefs(uri));
+  }
+
+  return uri.forget();
+}
+
 NS_IMETHODIMP
 nsImageLoadingContent::LoadImageWithChannel(nsIChannel* aChannel,
                                             nsIStreamListener** aListener)

dom/base/nsImageLoadingContent.h

@@ -70,6 +70,7 @@ public:
   int32_t
     GetRequestType(imgIRequest* aRequest, mozilla::ErrorResult& aError);
   already_AddRefed<nsIURI> GetCurrentURI(mozilla::ErrorResult& aError);
+  already_AddRefed<nsIURI> GetCurrentRequestFinalURI();
   void ForceReload(const mozilla::dom::Optional<bool>& aNotify,
                    mozilla::ErrorResult& aError);
 

dom/base/test/file_domwindowutils_animation.html

@@ -85,9 +85,10 @@ function test_getUnanimatedComputedStyle() {
       }
 
       addStyle([cssAnimationStyle,
-                ".pseudo::before { animation: cssanimation 1s; content: ''}"]);
+                ".pseudo::before { content: '' }",
+                ".animation::before { animation: cssanimation 1s }"]);
       const pseudoAnimation = target => {
-        target.classList.add("pseudo");
+        target.classList.add("animation");
         return target.getAnimations({ subtree: true })[0];
       }
       checkUnanimatedComputedStyle(property, initialStyle, "::before",
@@ -97,21 +98,51 @@ function test_getUnanimatedComputedStyle() {
     });
   });
 
+  const div = document.createElement("div");
+  document.body.appendChild(div);
+
   SimpleTest.doesThrow(
-    () => utils.getUnanimatedComputedStyle(div, null, "background"),
+    () => utils.getUnanimatedComputedStyle(div, null, "background", utils.FLUSH_NONE),
     "NS_ERROR_INVALID_ARG",
     "Shorthand property should throw");
 
   SimpleTest.doesThrow(
-    () => utils.getUnanimatedComputedStyle(div, null, "invalid"),
+    () => utils.getUnanimatedComputedStyle(div, null, "invalid", utils.FLUSH_NONE),
     "NS_ERROR_INVALID_ARG",
     "Invalid property should throw");
 
   SimpleTest.doesThrow(
-    () => utils.getUnanimatedComputedStyle(null, null, "opacity"),
+    () => utils.getUnanimatedComputedStyle(null, null, "opacity", utils.FLUSH_NONE),
     "NS_ERROR_INVALID_ARG",
     "Null element should throw");
 
+  SimpleTest.doesThrow(
+    () => utils.getUnanimatedComputedStyle(div, null, "opacity", utils.FLUSH_LAYOUT),
+    "NS_ERROR_INVALID_ARG",
+    "FLUSH_LAYOUT option should throw");
+
+  SimpleTest.doesThrow(
+    () => utils.getUnanimatedComputedStyle(div, null, "opacity", utils.FLUSH_DISPLAY),
+    "NS_ERROR_INVALID_ARG",
+    "FLUSH_DISPLAY option should throw");
+
+  if (utils.isStyledByServo) {
+    // Flush styles since getUnanimatedComputedStyle flushes pending styles even
+    // with FLUSH_NONE option if the element hasn't yet styled.
+    getComputedStyle(div).opacity;
+
+    div.style.opacity = "0";
+    is(utils.getUnanimatedComputedStyle(div, null, "opacity", utils.FLUSH_NONE),
+       "1",
+       "getUnanimatedComputedStyle with FLUSH_NONE should not flush pending styles");
+
+    is(utils.getUnanimatedComputedStyle(div, null, "opacity", utils.FLUSH_STYLE),
+       "0",
+       "getUnanimatedComputedStyle with FLUSH_STYLE should flush pending styles");
+  }
+
+  div.remove();
+
   next();
   window.close();
 }
@@ -126,8 +157,11 @@ function checkUnanimatedComputedStyle(property, initialStyle, pseudoType,
   if (initialStyle) {
     div.style[property] = initialStyle;
   }
+  if (pseudoType) {
+    div.classList.add("pseudo");
+  }
 
-  is(utils.getUnanimatedComputedStyle(div, pseudoType, property),
+  is(utils.getUnanimatedComputedStyle(div, pseudoType, property, utils.FLUSH_STYLE),
      expectedBeforeAnimation,
      `'${ property }' property with '${ initialStyle }' style `
      + `should be '${ expectedBeforeAnimation }' `
@@ -135,7 +169,7 @@ function checkUnanimatedComputedStyle(property, initialStyle, pseudoType,
 
   const animation = animate(div);
   animation.currentTime = 500;
-  is(utils.getUnanimatedComputedStyle(div, pseudoType, property),
+  is(utils.getUnanimatedComputedStyle(div, pseudoType, property, utils.FLUSH_STYLE),
      expectedDuringAnimation,
      `'${ property }' property with '${ initialStyle }' style `
      + `should be '${ expectedDuringAnimation }' `

dom/bindings/Codegen.py

@@ -7865,7 +7865,7 @@ class CGPerSignatureCall(CGThing):
                 if (CustomElementRegistry::IsCustomElementEnabled()) {
                   CustomElementReactionsStack* reactionsStack = GetCustomElementReactionsStack(${obj});
                   if (reactionsStack) {
-                    ceReaction.emplace(reactionsStack);
+                    ceReaction.emplace(reactionsStack, cx);
                   }
                 }
                 """, obj=objectName)))

dom/browser-element/BrowserElementChildPreload.js

@@ -862,8 +862,8 @@ BrowserElementChild.prototype = {
               documentURI: documentURI,
               text: elem.textContent.substring(0, kLongestReturnedString)};
     }
-    if (elem instanceof Ci.nsIImageLoadingContent && elem.currentURI) {
-      return {uri: elem.currentURI.spec, documentURI: documentURI};
+    if (elem instanceof Ci.nsIImageLoadingContent && elem.currentRequestFinalURI) {
+      return {uri: elem.currentRequestFinalURI.spec, documentURI: documentURI};
     }
     if (ChromeUtils.getClassName(elem) === "HTMLImageElement") {
       return {uri: elem.src, documentURI: documentURI};

dom/file/nsHostObjectURI.cpp

@@ -230,6 +230,20 @@ nsHostObjectURI::EqualsInternal(nsIURI* aOther,
   return NS_OK;
 }
 
+NS_IMPL_ISUPPORTS(nsHostObjectURI::Mutator, nsIURISetters, nsIURIMutator)
+
+NS_IMETHODIMP
+nsHostObjectURI::Mutate(nsIURIMutator** aMutator)
+{
+    RefPtr<nsHostObjectURI::Mutator> mutator = new nsHostObjectURI::Mutator();
+    nsresult rv = mutator->InitFromURI(this);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+    mutator.forget(aMutator);
+    return NS_OK;
+}
+
 // nsIClassInfo methods:
 NS_IMETHODIMP
 nsHostObjectURI::GetInterfaces(uint32_t *count, nsIID * **array)

dom/file/nsHostObjectURI.h

@@ -67,6 +67,8 @@ public:
     return url;
   }
 
+  NS_IMETHOD Mutate(nsIURIMutator * *_retval) override;
+
   void ForgetBlobImpl();
 
   nsCOMPtr<nsIPrincipal> mPrincipal;
@@ -74,6 +76,22 @@ public:
 
 protected:
   virtual ~nsHostObjectURI() {}
+
+public:
+  class Mutator
+    : public nsIURIMutator
+    , public BaseURIMutator<nsHostObjectURI>
+  {
+    NS_DECL_ISUPPORTS
+    NS_FORWARD_SAFE_NSIURISETTERS(mURI)
+    NS_DEFINE_NSIMUTATOR_COMMON
+
+    explicit Mutator() { }
+  private:
+    virtual ~Mutator() { }
+
+    friend class nsHostObjectURI;
+  };
 };
 
 #define NS_HOSTOBJECTURI_CID \

dom/interfaces/base/nsIDOMWindowUtils.idl

@@ -990,6 +990,7 @@ interface nsIDOMWindowUtils : nsISupports {
    */
   nsIDOMClientRect getBoundsWithoutFlushing(in nsIDOMElement aElement);
 
+  const long FLUSH_NONE = -1;
   const long FLUSH_STYLE = 0;
   const long FLUSH_LAYOUT = 1;
   const long FLUSH_DISPLAY = 2;
@@ -1581,10 +1582,13 @@ interface nsIDOMWindowUtils : nsISupports {
    * @param aElement - A target element
    * @param aPseudoElement - A pseudo type (e.g. '::before' or null)
    * @param aProperty - A longhand CSS property (e.g. 'background-color')
+   * @param aFlushType - FLUSH_NONE if any pending styles should not happen,
+   *                     FLUSH_STYLE to flush pending styles.
    */
   AString getUnanimatedComputedStyle(in nsIDOMElement aElement,
                                      in AString aPseudoElement,
-                                     in AString aProperty);
+                                     in AString aProperty,
+                                     in long aFlushType);
 
   /**
    * Get the type of the currently focused html input, if any.

dom/jsurl/nsJSProtocolHandler.cpp

@@ -1375,6 +1375,20 @@ nsJSURI::StartClone(mozilla::net::nsSimpleURI::RefHandlingEnum refHandlingMode,
     return url;
 }
 
+NS_IMPL_ISUPPORTS(nsJSURI::Mutator, nsIURISetters, nsIURIMutator)
+
+NS_IMETHODIMP
+nsJSURI::Mutate(nsIURIMutator** aMutator)
+{
+    RefPtr<nsJSURI::Mutator> mutator = new nsJSURI::Mutator();
+    nsresult rv = mutator->InitFromURI(this);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+    mutator.forget(aMutator);
+    return NS_OK;
+}
+
 /* virtual */ nsresult
 nsJSURI::EqualsInternal(nsIURI* aOther,
                         mozilla::net::nsSimpleURI::RefHandlingEnum aRefHandlingMode,

dom/jsurl/nsJSProtocolHandler.h

@@ -81,6 +81,7 @@ public:
     // nsIURI overrides
     virtual mozilla::net::nsSimpleURI* StartClone(RefHandlingEnum refHandlingMode,
                                                   const nsACString& newRef) override;
+    NS_IMETHOD Mutate(nsIURIMutator * *_retval) override;
 
     // nsISerializable overrides
     NS_IMETHOD Read(nsIObjectInputStream* aStream) override;
@@ -102,6 +103,22 @@ protected:
                                     bool* result) override;
 private:
     nsCOMPtr<nsIURI> mBaseURI;
+
+public:
+    class Mutator
+        : public nsIURIMutator
+        , public BaseURIMutator<nsJSURI>
+    {
+        NS_DECL_ISUPPORTS
+        NS_FORWARD_SAFE_NSIURISETTERS(mURI)
+        NS_DEFINE_NSIMUTATOR_COMMON
+
+        explicit Mutator() { }
+    private:
+        virtual ~Mutator() { }
+
+        friend class nsJSURI;
+    };
 };
 
 #endif /* nsJSProtocolHandler_h___ */

dom/media/ChannelMediaResource.cpp

@@ -192,6 +192,7 @@ ChannelMediaResource::OnStartRequest(nsIRequest* aRequest,
 
   nsCOMPtr<nsIHttpChannel> hc = do_QueryInterface(aRequest);
   bool seekable = false;
+  int64_t length = -1;
   int64_t startOffset = aRequestOffset;
 
   if (hc) {
@@ -275,7 +276,7 @@ ChannelMediaResource::OnStartRequest(nsIRequest* aRequest,
     if (aRequestOffset == 0 && contentLength >= 0 &&
         (responseStatus == HTTP_OK_CODE ||
          responseStatus == HTTP_PARTIAL_RESPONSE_CODE)) {
-      mCacheStream.NotifyDataLength(contentLength);
+      length = contentLength;
     }
     // XXX we probably should examine the Content-Range header in case
     // the server gave us a range which is not quite what we asked for
@@ -294,7 +295,7 @@ ChannelMediaResource::OnStartRequest(nsIRequest* aRequest,
   // any consumer can see the new data.
   UpdatePrincipal();
 
-  mCacheStream.NotifyDataStarted(mLoadID, startOffset, seekable);
+  mCacheStream.NotifyDataStarted(mLoadID, startOffset, seekable, length);
   mIsTransportSeekable = seekable;
   mChannelStatistics.Start();
 
@@ -303,7 +304,6 @@ ChannelMediaResource::OnStartRequest(nsIRequest* aRequest,
   // Fires an initial progress event.
   owner->DownloadProgressed();
 
-  // TODO: Don't turn this on until we fix all data races.
   nsCOMPtr<nsIThreadRetargetableRequest> retarget;
   if (Preferences::GetBool("media.omt_data_delivery.enabled", false) &&
       (retarget = do_QueryInterface(aRequest))) {
@@ -417,12 +417,25 @@ ChannelMediaResource::CopySegmentToCache(nsIInputStream* aInStream,
                                          uint32_t aCount,
                                          uint32_t* aWriteCount)
 {
-  Closure* closure = static_cast<Closure*>(aClosure);
-  closure->mResource->mCacheStream.NotifyDataReceived(
-    closure->mLoadID,
-    aCount,
-    reinterpret_cast<const uint8_t*>(aFromSegment));
   *aWriteCount = aCount;
+  Closure* closure = static_cast<Closure*>(aClosure);
+  MediaCacheStream* cacheStream = &closure->mResource->mCacheStream;
+  if (cacheStream->OwnerThread()->IsOnCurrentThread()) {
+    cacheStream->NotifyDataReceived(
+      closure->mLoadID, aCount, reinterpret_cast<const uint8_t*>(aFromSegment));
+    return NS_OK;
+  }
+
+  RefPtr<ChannelMediaResource> self = closure->mResource;
+  uint32_t loadID = closure->mLoadID;
+  UniquePtr<uint8_t[]> data = MakeUnique<uint8_t[]>(aCount);
+  memcpy(data.get(), aFromSegment, aCount);
+  cacheStream->OwnerThread()->Dispatch(NS_NewRunnableFunction(
+    "MediaCacheStream::NotifyDataReceived",
+    [ self, loadID, data = Move(data), aCount ]() {
+      self->mCacheStream.NotifyDataReceived(loadID, aCount, data.get());
+    }));
+
   return NS_OK;
 }
 

dom/media/MediaCache.cpp

@@ -1933,16 +1933,6 @@ MediaCache::NoteSeek(MediaCacheStream* aStream, int64_t aOldOffset)
   }
 }
 
-void
-MediaCacheStream::NotifyDataLength(int64_t aLength)
-{
-  NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
-  LOG("Stream %p DataLength: %" PRId64, this, aLength);
-
-  ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
-  mStreamLength = aLength;
-}
-
 void
 MediaCacheStream::NotifyLoadID(uint32_t aLoadID)
 {
@@ -1954,16 +1944,24 @@ MediaCacheStream::NotifyLoadID(uint32_t aLoadID)
 void
 MediaCacheStream::NotifyDataStarted(uint32_t aLoadID,
                                     int64_t aOffset,
-                                    bool aSeekable)
+                                    bool aSeekable,
+                                    int64_t aLength)
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   MOZ_ASSERT(aLoadID > 0);
-  LOG("Stream %p DataStarted: %" PRId64 " aLoadID=%u", this, aOffset, aLoadID);
+  LOG("Stream %p DataStarted: %" PRId64 " aLoadID=%u aLength=%" PRId64,
+      this,
+      aOffset,
+      aLoadID,
+      aLength);
 
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   NS_WARNING_ASSERTION(aOffset == mSeekTarget || aOffset == mChannelOffset,
                        "Server is giving us unexpected offset");
   MOZ_ASSERT(aOffset >= 0);
+  if (aLength >= 0) {
+    mStreamLength = aLength;
+  }
   mChannelOffset = aOffset;
   if (mStreamLength >= 0) {
     // If we started reading at a certain offset, then for sure
@@ -2006,8 +2004,8 @@ MediaCacheStream::NotifyDataReceived(uint32_t aLoadID,
                                      uint32_t aCount,
                                      const uint8_t* aData)
 {
+  MOZ_ASSERT(OwnerThread()->IsOnCurrentThread());
   MOZ_ASSERT(aLoadID > 0);
-  // This might happen off the main thread.
 
   ReentrantMonitorAutoEnter mon(mMediaCache->GetReentrantMonitor());
   if (mClosed) {
@@ -2727,7 +2725,7 @@ MediaCacheStream::Init(int64_t aContentLength)
 
   if (aContentLength > 0) {
     uint32_t length = uint32_t(std::min(aContentLength, int64_t(UINT32_MAX)));
-    LOG("MediaCacheStream::NotifyDataLength(this=%p) "
+    LOG("MediaCacheStream::Init(this=%p) "
         "MEDIACACHESTREAM_NOTIFIED_LENGTH=%" PRIu32,
         this,
         length);

dom/media/MediaCache.h

@@ -230,8 +230,24 @@ public:
 
   // These callbacks are called on the main thread by the client
   // when data has been received via the channel.
-  // Tells the cache what the server said the data length is going to be.
-  // The actual data length may be greater (we receive more data than
+
+  // Notifies the cache that a load has begun. We pass the offset
+  // because in some cases the offset might not be what the cache
+  // requested. In particular we might unexpectedly start providing
+  // data at offset 0. This need not be called if the offset is the
+  // offset that the cache requested in
+  // ChannelMediaResource::CacheClientSeek. This can be called at any
+  // time by the client, not just after a CacheClientSeek.
+  //
+  // aSeekable tells us whether the stream is seekable or not. Non-seekable
+  // streams will always pass 0 for aOffset to CacheClientSeek. This should only
+  // be called while the stream is at channel offset 0. Seekability can
+  // change during the lifetime of the MediaCacheStream --- every time
+  // we do an HTTP load the seekability may be different (and sometimes
+  // is, in practice, due to the effects of caching proxies).
+  //
+  // aLength tells the cache what the server said the data length is going to
+  // be. The actual data length may be greater (we receive more data than
   // specified) or smaller (the stream ends before we reach the given
   // length), because servers can lie. The server's reported data length
   // *and* the actual data length can even vary over time because a
@@ -241,21 +257,10 @@ public:
   // data available based on an incorrect reported length. Seeks relative
   // EOF also depend on the reported length if we haven't managed to
   // read the whole stream yet.
-  void NotifyDataLength(int64_t aLength);
-  // Notifies the cache that a load has begun. We pass the offset
-  // because in some cases the offset might not be what the cache
-  // requested. In particular we might unexpectedly start providing
-  // data at offset 0. This need not be called if the offset is the
-  // offset that the cache requested in
-  // ChannelMediaResource::CacheClientSeek. This can be called at any
-  // time by the client, not just after a CacheClientSeek.
-  // aSeekable tells us whether the stream is seekable or not. Non-seekable
-  // streams will always pass 0 for aOffset to CacheClientSeek. This should only
-  // be called while the stream is at channel offset 0. Seekability can
-  // change during the lifetime of the MediaCacheStream --- every time
-  // we do an HTTP load the seekability may be different (and sometimes
-  // is, in practice, due to the effects of caching proxies).
-  void NotifyDataStarted(uint32_t aLoadID, int64_t aOffset, bool aSeekable);
+  void NotifyDataStarted(uint32_t aLoadID,
+                         int64_t aOffset,
+                         bool aSeekable,
+                         int64_t aLength);
   // Notifies the cache that data has been received. The stream already
   // knows the offset because data is received in sequence and
   // the starting offset is known via NotifyDataStarted or because
@@ -287,7 +292,7 @@ public:
   // while the stream is pinned.
   void Pin();
   void Unpin();
-  // See comments above for NotifyDataLength about how the length
+  // See comments above for NotifyDataStarted about how the length
   // can vary over time. Returns -1 if no length is known. Returns the
   // reported length if we haven't got any better information. If
   // the stream ended normally we return the length we actually got.

dom/media/MediaPrefs.cpp

@@ -69,6 +69,20 @@ void MediaPrefs::PrefAddVarCache(float* aVariable,
   Preferences::AddFloatVarCache(aVariable, aPref, aDefault);
 }
 
+void MediaPrefs::PrefAddVarCache(AtomicBool* aVariable,
+                                 const char* aPref,
+                                 bool aDefault)
+{
+  Preferences::AddAtomicBoolVarCache(aVariable, aPref, aDefault);
+}
+
+void MediaPrefs::PrefAddVarCache(AtomicInt32* aVariable,
+                                 const char* aPref,
+                                 int32_t aDefault)
+{
+  Preferences::AddAtomicIntVarCache(aVariable, aPref, aDefault);
+}
+
 void MediaPrefs::PrefAddVarCache(AtomicUint32* aVariable,
                                  const char* aPref,
                                  uint32_t aDefault)

dom/media/MediaPrefs.h

@@ -49,6 +49,8 @@ template<class T> class StaticAutoPtr;
 
 class MediaPrefs final
 {
+  typedef Atomic<bool, Relaxed> AtomicBool;
+  typedef Atomic<int32_t, Relaxed> AtomicInt32;
   typedef Atomic<uint32_t, Relaxed> AtomicUint32;
 
   template <typename T>
@@ -215,6 +217,8 @@ private:
   static void PrefAddVarCache(int32_t*, const char*, int32_t);
   static void PrefAddVarCache(uint32_t*, const char*, uint32_t);
   static void PrefAddVarCache(float*, const char*, float);
+  static void PrefAddVarCache(AtomicBool*, const char*, bool);
+  static void PrefAddVarCache(AtomicInt32*, const char*, int32_t);
   static void PrefAddVarCache(AtomicUint32*, const char*, uint32_t);
 
   static void AssertMainThread();

dom/webidl/HTMLImageElement.webidl

@@ -102,6 +102,10 @@ interface MozImageLoadingContent {
   long getRequestType(imgIRequest aRequest);
   [ChromeOnly,Throws]
   readonly attribute URI? currentURI;
+  // Gets the final URI of the current request, if available.
+  // Otherwise, returns null.
+  [ChromeOnly]
+  readonly attribute URI? currentRequestFinalURI;
   [ChromeOnly,Throws]
   void forceReload(optional boolean aNotify);
   [ChromeOnly]

editor/libeditor/EditorBase.cpp

@@ -1419,15 +1419,17 @@ EditorBase::SetSpellcheckUserOverride(bool enable)
 
 already_AddRefed<Element>
 EditorBase::CreateNode(nsAtom* aTag,
-                       EditorRawDOMPoint& aPointToInsert)
+                       const EditorRawDOMPoint& aPointToInsert)
 {
   MOZ_ASSERT(aTag);
   MOZ_ASSERT(aPointToInsert.IsSetAndValid());
 
-  // XXX We need to offset at new node to mRangeUpdater.  Therefore, we need
+  EditorRawDOMPoint pointToInsert(aPointToInsert);
+
+  // XXX We need offset at new node for mRangeUpdater.  Therefore, we need
   //     to compute the offset now but this is expensive.  So, if it's possible,
   //     we need to redesign mRangeUpdater as avoiding using indices.
-  int32_t offset = static_cast<int32_t>(aPointToInsert.Offset());
+  int32_t offset = static_cast<int32_t>(pointToInsert.Offset());
 
   AutoRules beginRulesSniffing(this, EditAction::createNode, nsIEditor::eNext);
 
@@ -1435,14 +1437,14 @@ EditorBase::CreateNode(nsAtom* aTag,
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->WillCreateNode(nsDependentAtomString(aTag),
-                               GetAsDOMNode(aPointToInsert.GetChildAtOffset()));
+                               GetAsDOMNode(pointToInsert.GetChildAtOffset()));
     }
   }
 
   nsCOMPtr<Element> ret;
 
   RefPtr<CreateElementTransaction> transaction =
-    CreateTxnForCreateElement(*aTag, aPointToInsert);
+    CreateTxnForCreateElement(*aTag, pointToInsert);
   nsresult rv = DoTransaction(transaction);
   if (NS_SUCCEEDED(rv)) {
     ret = transaction->GetNewNode();
@@ -1450,10 +1452,10 @@ EditorBase::CreateNode(nsAtom* aTag,
     // Now, aPointToInsert may be invalid.  I.e., ChildAtOffset() keeps
     // referring the next sibling of new node but Offset() refers the
     // new node.  Let's make refer the new node.
-    aPointToInsert.Set(ret);
+    pointToInsert.Set(ret);
   }
 
-  mRangeUpdater.SelAdjCreateNode(aPointToInsert.Container(), offset);
+  mRangeUpdater.SelAdjCreateNode(pointToInsert.Container(), offset);
 
   {
     AutoActionListenerArray listeners(mActionListeners);
@@ -1516,49 +1518,72 @@ EditorBase::SplitNode(nsIDOMNode* aNode,
                       nsIDOMNode** aNewLeftNode)
 {
   nsCOMPtr<nsIContent> node = do_QueryInterface(aNode);
-  NS_ENSURE_STATE(node);
-  ErrorResult rv;
-  nsCOMPtr<nsIContent> newNode = SplitNode(*node, aOffset, rv);
+  if (NS_WARN_IF(!node)) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  int32_t offset = std::min(std::max(aOffset, 0),
+                            static_cast<int32_t>(node->Length()));
+  ErrorResult error;
+  nsCOMPtr<nsIContent> newNode =
+    SplitNode(EditorRawDOMPoint(node, offset), error);
   *aNewLeftNode = GetAsDOMNode(newNode.forget().take());
-  return rv.StealNSResult();
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  return NS_OK;
 }
 
-nsIContent*
-EditorBase::SplitNode(nsIContent& aNode,
-                      int32_t aOffset,
-                      ErrorResult& aResult)
+already_AddRefed<nsIContent>
+EditorBase::SplitNode(const EditorRawDOMPoint& aStartOfRightNode,
+                      ErrorResult& aError)
 {
+  if (NS_WARN_IF(!aStartOfRightNode.IsSet()) ||
+      NS_WARN_IF(!aStartOfRightNode.Container()->IsContent())) {
+    aError.Throw(NS_ERROR_INVALID_ARG);
+    return nullptr;
+  }
+  MOZ_ASSERT(aStartOfRightNode.IsSetAndValid());
+
   AutoRules beginRulesSniffing(this, EditAction::splitNode, nsIEditor::eNext);
 
+  // Different from CreateNode(), we need offset at start of right node only
+  // for WillSplitNode() since the offset is always same as the length of new
+  // left node.
   {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
-      listener->WillSplitNode(aNode.AsDOMNode(), aOffset);
+      // XXX Unfortunately, we need to compute offset here because the container
+      //     may be a data node like text node.  However, nobody implements this
+      //     method actually.  So, we should get rid of this in a follow up bug.
+      listener->WillSplitNode(aStartOfRightNode.Container()->AsDOMNode(),
+                              aStartOfRightNode.Offset());
     }
   }
 
   RefPtr<SplitNodeTransaction> transaction =
-    CreateTxnForSplitNode(aNode, aOffset);
-  aResult = DoTransaction(transaction);
+    CreateTxnForSplitNode(aStartOfRightNode);
+  aError = DoTransaction(transaction);
 
-  nsCOMPtr<nsIContent> newNode = aResult.Failed() ? nullptr
-                                                  : transaction->GetNewNode();
+  nsCOMPtr<nsIContent> newNode = transaction->GetNewNode();
+  NS_WARNING_ASSERTION(newNode, "Failed to create a new left node");
 
-  mRangeUpdater.SelAdjSplitNode(aNode, aOffset, newNode);
+  mRangeUpdater.SelAdjSplitNode(*aStartOfRightNode.Container()->AsContent(),
+                                newNode);
 
-  nsresult rv = aResult.StealNSResult();
   {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
-      listener->DidSplitNode(aNode.AsDOMNode(), aOffset, GetAsDOMNode(newNode),
-                             rv);
+      listener->DidSplitNode(aStartOfRightNode.Container()->AsDOMNode(),
+                             GetAsDOMNode(newNode));
     }
   }
-  // Note: result might be a success code, so we can't use Throw() to
-  // set it on aResult.
-  aResult = rv;
 
-  return newNode;
+  if (NS_WARN_IF(aError.Failed())) {
+    return nullptr;
+  }
+
+  return newNode.forget();
 }
 
 NS_IMETHODIMP
@@ -2873,11 +2898,11 @@ EditorBase::CreateTxnForDeleteText(nsGenericDOMDataNode& aCharData,
 }
 
 already_AddRefed<SplitNodeTransaction>
-EditorBase::CreateTxnForSplitNode(nsIContent& aNode,
-                                  uint32_t aOffset)
+EditorBase::CreateTxnForSplitNode(const EditorRawDOMPoint& aStartOfRightNode)
 {
+  MOZ_ASSERT(aStartOfRightNode.IsSetAndValid());
   RefPtr<SplitNodeTransaction> transaction =
-    new SplitNodeTransaction(*this, aNode, aOffset);
+    new SplitNodeTransaction(*this, aStartOfRightNode);
   return transaction.forget();
 }
 
@@ -2904,18 +2929,32 @@ struct SavedRange final
   int32_t mEndOffset;
 };
 
-nsresult
-EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
-                          int32_t aOffset,
-                          nsIContent& aNewLeftNode)
+void
+EditorBase::SplitNodeImpl(const EditorDOMPoint& aStartOfRightNode,
+                          nsIContent& aNewLeftNode,
+                          ErrorResult& aError)
 {
+  if (NS_WARN_IF(aError.Failed())) {
+    return;
+  }
+
+  // XXX Perhaps, aStartOfRightNode may be invalid if this is a redo
+  //     operation after modifying DOM node with JS.
+  if (NS_WARN_IF(!aStartOfRightNode.IsSet())) {
+    aError.Throw(NS_ERROR_INVALID_ARG);
+    return;
+  }
+  MOZ_ASSERT(aStartOfRightNode.IsSetAndValid());
+
   // Remember all selection points.
   AutoTArray<SavedRange, 10> savedRanges;
   for (SelectionType selectionType : kPresentSelectionTypes) {
     SavedRange range;
     range.mSelection = GetSelection(selectionType);
-    if (selectionType == SelectionType::eNormal) {
-      NS_ENSURE_TRUE(range.mSelection, NS_ERROR_NULL_POINTER);
+    if (NS_WARN_IF(!range.mSelection &&
+                   selectionType == SelectionType::eNormal)) {
+      aError.Throw(NS_ERROR_FAILURE);
+      return;
     } else if (!range.mSelection) {
       // For non-normal selections, skip over the non-existing ones.
       continue;
@@ -2924,6 +2963,8 @@ EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
     for (uint32_t j = 0; j < range.mSelection->RangeCount(); ++j) {
       RefPtr<nsRange> r = range.mSelection->GetRangeAt(j);
       MOZ_ASSERT(r->IsPositioned());
+      // XXX Looks like that SavedRange should have mStart and mEnd which
+      //     are RangeBoundary.  Then, we can avoid to compute offset here.
       range.mStartContainer = r->GetStartContainer();
       range.mStartOffset = r->StartOffset();
       range.mEndContainer = r->GetEndContainer();
@@ -2933,54 +2974,69 @@ EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
     }
   }
 
-  nsCOMPtr<nsINode> parent = aExistingRightNode.GetParentNode();
-  NS_ENSURE_TRUE(parent, NS_ERROR_NULL_POINTER);
-
-  ErrorResult rv;
-  nsCOMPtr<nsINode> refNode = &aExistingRightNode;
-  parent->InsertBefore(aNewLeftNode, refNode, rv);
-  NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
-
-  // Split the children between the two nodes.  At this point,
-  // aExistingRightNode has all the children.  Move all the children whose
-  // index is < aOffset to aNewLeftNode.
-  if (aOffset < 0) {
-    // This means move no children
-    return NS_OK;
+  nsCOMPtr<nsINode> parent = aStartOfRightNode.Container()->GetParentNode();
+  if (NS_WARN_IF(!parent)) {
+    aError.Throw(NS_ERROR_FAILURE);
+    return;
   }
 
-  // If it's a text node, just shuffle around some text
-  if (aExistingRightNode.GetAsText() && aNewLeftNode.GetAsText()) {
-    // Fix right node
-    nsAutoString leftText;
-    aExistingRightNode.GetAsText()->SubstringData(0, aOffset, leftText);
-    aExistingRightNode.GetAsText()->DeleteData(0, aOffset);
-    // Fix left node
-    aNewLeftNode.GetAsText()->SetData(leftText);
-  } else {
-    // Otherwise it's an interior node, so shuffle around the children. Go
-    // through list backwards so deletes don't interfere with the iteration.
-    nsCOMPtr<nsINodeList> childNodes = aExistingRightNode.ChildNodes();
-    for (int32_t i = aOffset - 1; i >= 0; i--) {
-      nsCOMPtr<nsIContent> childNode = childNodes->Item(i);
-      if (childNode) {
-        aExistingRightNode.RemoveChild(*childNode, rv);
-        if (!rv.Failed()) {
-          nsCOMPtr<nsIContent> firstChild = aNewLeftNode.GetFirstChild();
-          aNewLeftNode.InsertBefore(*childNode, firstChild, rv);
+  parent->InsertBefore(aNewLeftNode, aStartOfRightNode.Container(),
+                       aError);
+  if (NS_WARN_IF(aError.Failed())) {
+    return;
+  }
+
+  // At this point, the existing right node has all the children.  Move all
+  // the children which are before aStartOfRightNode.
+  if (!aStartOfRightNode.IsStartOfContainer()) {
+    // If it's a text node, just shuffle around some text
+    Text* rightAsText = aStartOfRightNode.Container()->GetAsText();
+    Text* leftAsText = aNewLeftNode.GetAsText();
+    if (rightAsText && leftAsText) {
+      // Fix right node
+      nsAutoString leftText;
+      rightAsText->SubstringData(0, aStartOfRightNode.Offset(),
+                                 leftText);
+      rightAsText->DeleteData(0, aStartOfRightNode.Offset());
+      // Fix left node
+      leftAsText->GetAsText()->SetData(leftText);
+    } else {
+      MOZ_DIAGNOSTIC_ASSERT(!rightAsText && !leftAsText);
+      // Otherwise it's an interior node, so shuffle around the children. Go
+      // through list backwards so deletes don't interfere with the iteration.
+      // FYI: It's okay to use raw pointer for caching existing right node since
+      //      it's already grabbed by aStartOfRightNode.
+      nsINode* existingRightNode = aStartOfRightNode.Container();
+      nsCOMPtr<nsINodeList> childNodes = existingRightNode->ChildNodes();
+      // XXX This is wrong loop range if some children has already gone.
+      //     This will be fixed by a later patch.
+      for (int32_t i = aStartOfRightNode.Offset() - 1; i >= 0; i--) {
+        nsCOMPtr<nsIContent> childNode = childNodes->Item(i);
+        MOZ_RELEASE_ASSERT(childNode);
+        existingRightNode->RemoveChild(*childNode, aError);
+        if (NS_WARN_IF(aError.Failed())) {
+          break;
         }
-      }
-      if (rv.Failed()) {
-        break;
+        nsCOMPtr<nsIContent> firstChild = aNewLeftNode.GetFirstChild();
+        aNewLeftNode.InsertBefore(*childNode, firstChild, aError);
+        NS_WARNING_ASSERTION(!aError.Failed(),
+          "Failed to insert a child which is removed from the right node into "
+          "the left node");
       }
     }
   }
 
+  // XXX Why do we ignore an error while moving nodes from the right node to
+  //     the left node?
+  aError.SuppressException();
+
   // Handle selection
   nsCOMPtr<nsIPresShell> ps = GetPresShell();
   if (ps) {
     ps->FlushPendingNotifications(FlushType::Frames);
   }
+  NS_WARNING_ASSERTION(!Destroyed(),
+    "The editor is destroyed during splitting a node");
 
   bool shouldSetSelection = GetShouldTxnSetSelection();
 
@@ -2991,11 +3047,16 @@ EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
 
     // If we have not seen the selection yet, clear all of its ranges.
     if (range.mSelection != previousSelection) {
-      nsresult rv = range.mSelection->RemoveAllRanges();
-      NS_ENSURE_SUCCESS(rv, rv);
+      range.mSelection->RemoveAllRanges(aError);
+      if (NS_WARN_IF(aError.Failed())) {
+        return;
+      }
       previousSelection = range.mSelection;
     }
 
+    // XXX Looks like that we don't need to modify normal selection here
+    //     because selection will be modified by the caller if
+    //     GetShouldTxnSetSelection() will return true.
     if (shouldSetSelection &&
         range.mSelection->Type() == SelectionType::eNormal) {
       // If the editor should adjust the selection, don't bother restoring
@@ -3004,19 +3065,21 @@ EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
     }
 
     // Split the selection into existing node and new node.
-    if (range.mStartContainer == &aExistingRightNode) {
-      if (range.mStartOffset < aOffset) {
+    if (range.mStartContainer == aStartOfRightNode.Container()) {
+      if (static_cast<uint32_t>(range.mStartOffset) <
+            aStartOfRightNode.Offset()) {
         range.mStartContainer = &aNewLeftNode;
       } else {
-        range.mStartOffset -= aOffset;
+        range.mStartOffset -= aStartOfRightNode.Offset();
       }
     }
 
-    if (range.mEndContainer == &aExistingRightNode) {
-      if (range.mEndOffset < aOffset) {
+    if (range.mEndContainer == aStartOfRightNode.Container()) {
+      if (static_cast<uint32_t>(range.mEndOffset) <
+            aStartOfRightNode.Offset()) {
         range.mEndContainer = &aNewLeftNode;
       } else {
-        range.mEndOffset -= aOffset;
+        range.mEndOffset -= aStartOfRightNode.Offset();
       }
     }
 
@@ -3026,19 +3089,18 @@ EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
                                        range.mEndContainer,
                                        range.mEndOffset,
                                        getter_AddRefs(newRange));
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = range.mSelection->AddRange(newRange);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      aError.Throw(rv);
+      return;
+    }
+    range.mSelection->AddRange(*newRange, aError);
+    if (NS_WARN_IF(aError.Failed())) {
+      return;
+    }
   }
 
-  if (shouldSetSelection) {
-    // Editor wants us to set selection at split point.
-    RefPtr<Selection> selection = GetSelection();
-    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
-    selection->Collapse(&aNewLeftNode, aOffset);
-  }
-
-  return NS_OK;
+  // We don't need to set selection here because the caller should do that
+  // in any case.
 }
 
 nsresult
@@ -3982,86 +4044,87 @@ EditorBase::IsPreformatted(nsIDOMNode* aNode,
   return NS_OK;
 }
 
-
-/**
- * This splits a node "deeply", splitting children as appropriate.  The place
- * to split is represented by a DOM point at {splitPointParent,
- * splitPointOffset}.  That DOM point must be inside aNode, which is the node
- * to split.  We return the offset in the parent of aNode where the split
- * terminates - where you would want to insert a new element, for instance, if
- * that's why you were splitting the node.
- *
- * -1 is returned on failure, in unlikely cases like the selection being
- * unavailable or cloning the node failing.  Make sure not to use the returned
- * offset for anything without checking that it's valid!  If you're not using
- * the offset, it's okay to ignore the return value.
- */
-int32_t
-EditorBase::SplitNodeDeep(nsIContent& aNode,
-                          nsIContent& aSplitPointParent,
-                          int32_t aSplitPointOffset,
-                          EmptyContainers aEmptyContainers,
-                          nsIContent** aOutLeftNode,
-                          nsIContent** aOutRightNode,
-                          nsCOMPtr<nsIContent>* ioChildAtSplitPointOffset)
+SplitNodeResult
+EditorBase::SplitNodeDeep(nsIContent& aMostAncestorToSplit,
+                          const EditorRawDOMPoint& aStartOfDeepestRightNode,
+                          SplitAtEdges aSplitAtEdges)
 {
-  MOZ_ASSERT(&aSplitPointParent == &aNode ||
-             EditorUtils::IsDescendantOf(aSplitPointParent, aNode));
-  int32_t offset = aSplitPointOffset;
+  MOZ_ASSERT(aStartOfDeepestRightNode.IsSetAndValid());
+  MOZ_ASSERT(aStartOfDeepestRightNode.Container() == &aMostAncestorToSplit ||
+             EditorUtils::IsDescendantOf(*aStartOfDeepestRightNode.Container(),
+                                         aMostAncestorToSplit));
 
-  nsCOMPtr<nsIContent> leftNode, rightNode;
-  OwningNonNull<nsIContent> nodeToSplit = aSplitPointParent;
+  if (NS_WARN_IF(!aStartOfDeepestRightNode.IsSet())) {
+    return SplitNodeResult(NS_ERROR_INVALID_ARG);
+  }
+
+  nsCOMPtr<nsIContent> newLeftNodeOfMostAncestor;
+  EditorDOMPoint atStartOfRightNode(aStartOfDeepestRightNode);
   while (true) {
+    // If we meet an orphan node before meeting aMostAncestorToSplit, we need
+    // to stop splitting.  This is a bug of the caller.
+    if (NS_WARN_IF(atStartOfRightNode.Container() != &aMostAncestorToSplit &&
+                   !atStartOfRightNode.Container()->GetParent())) {
+      return SplitNodeResult(NS_ERROR_FAILURE);
+    }
+
     // Need to insert rules code call here to do things like not split a list
     // if you are after the last <li> or before the first, etc.  For now we
     // just have some smarts about unneccessarily splitting text nodes, which
     // should be universal enough to put straight in this EditorBase routine.
 
-    bool didSplit = false;
-
-    if ((aEmptyContainers == EmptyContainers::yes &&
-         !nodeToSplit->GetAsText()) ||
-        (offset && offset != (int32_t)nodeToSplit->Length())) {
-      didSplit = true;
-      ErrorResult rv;
-      nsCOMPtr<nsIContent> newLeftNode = SplitNode(nodeToSplit, offset, rv);
-      NS_ENSURE_TRUE(!NS_FAILED(rv.StealNSResult()), -1);
-
-      rightNode = nodeToSplit;
-      leftNode = newLeftNode;
+    if (NS_WARN_IF(!atStartOfRightNode.Container()->IsContent())) {
+      return SplitNodeResult(NS_ERROR_FAILURE);
     }
+    nsIContent* currentRightNode = atStartOfRightNode.Container()->AsContent();
 
-    NS_ENSURE_TRUE(nodeToSplit->GetParent(), -1);
-    OwningNonNull<nsIContent> parentNode = *nodeToSplit->GetParent();
+    // If the split point is middle of the node or the node is not a text node
+    // and we're allowed to create empty element node, split it.
+    if ((aSplitAtEdges == SplitAtEdges::eAllowToCreateEmptyContainer &&
+         !atStartOfRightNode.Container()->GetAsText()) ||
+        (!atStartOfRightNode.IsStartOfContainer() &&
+         !atStartOfRightNode.IsEndOfContainer())) {
+      ErrorResult error;
+      nsCOMPtr<nsIContent> newLeftNode =
+        SplitNode(atStartOfRightNode.AsRaw(), error);
+      if (NS_WARN_IF(error.Failed())) {
+        return SplitNodeResult(NS_ERROR_FAILURE);
+      }
 
-    if (!didSplit && offset) {
-      // Must be "end of text node" case, we didn't split it, just move past it
-      offset = parentNode->IndexOf(nodeToSplit) + 1;
-      leftNode = nodeToSplit;
-    } else {
-      offset = parentNode->IndexOf(nodeToSplit);
-      rightNode = nodeToSplit;
+      if (currentRightNode == &aMostAncestorToSplit) {
+        // Actually, we split aMostAncestorToSplit.
+        return SplitNodeResult(newLeftNode, &aMostAncestorToSplit);
+      }
+
+      // Then, try to split its parent before current node.
+      atStartOfRightNode.Set(currentRightNode);
     }
+    // If the split point is end of the node and it is a text node or we're not
+    // allowed to create empty container node, try to split its parent after it.
+    else if (!atStartOfRightNode.IsStartOfContainer()) {
+      if (currentRightNode == &aMostAncestorToSplit) {
+        return SplitNodeResult(&aMostAncestorToSplit, nullptr);
+      }
 
-    if (nodeToSplit == &aNode) {
-      // we split all the way up to (and including) aNode; we're done
-      break;
+      // Try to split its parent after current node.
+      atStartOfRightNode.Set(currentRightNode);
+      DebugOnly<bool> advanced = atStartOfRightNode.AdvanceOffset();
+      NS_WARNING_ASSERTION(advanced,
+        "Failed to advance offset after current node");
     }
+    // If the split point is start of the node and it is a text node or we're
+    // not allowed to create empty container node, try to split its parent.
+    else {
+      if (currentRightNode == &aMostAncestorToSplit) {
+        return SplitNodeResult(nullptr, &aMostAncestorToSplit);
+      }
 
-    nodeToSplit = parentNode;
+      // Try to split its parent before current node.
+      atStartOfRightNode.Set(currentRightNode);
+    }
   }
 
-  if (aOutLeftNode) {
-    leftNode.forget(aOutLeftNode);
-  }
-  if (aOutRightNode) {
-    rightNode.forget(aOutRightNode);
-  }
-  if (ioChildAtSplitPointOffset) {
-    *ioChildAtSplitPointOffset = nodeToSplit;
-  }
-
-  return offset;
+  return SplitNodeResult(NS_ERROR_FAILURE);
 }
 
 /**
@@ -4319,42 +4382,60 @@ EditorBase::DeleteSelectionAndPrepareToCreateNode()
   nsCOMPtr<nsINode> node = selection->GetAnchorNode();
   MOZ_ASSERT(node, "Selection has no ranges in it");
 
-  if (node && node->IsNodeOfType(nsINode::eDATA_NODE)) {
-    NS_ASSERTION(node->GetParentNode(),
-                 "It's impossible to insert into chardata with no parent -- "
-                 "fix the caller");
-    NS_ENSURE_STATE(node->GetParentNode());
+  if (!node || !node->IsNodeOfType(nsINode::eDATA_NODE)) {
+    return NS_OK;
+  }
 
-    uint32_t offset = selection->AnchorOffset();
+  NS_ASSERTION(node->GetParentNode(),
+               "It's impossible to insert into chardata with no parent -- "
+               "fix the caller");
+  NS_ENSURE_STATE(node->GetParentNode());
 
-    if (!offset) {
-      EditorRawDOMPoint atNode(node);
-      if (NS_WARN_IF(!atNode.IsSetAndValid())) {
-        return NS_ERROR_FAILURE;
-      }
-      nsresult rv = selection->Collapse(atNode);
-      MOZ_ASSERT(NS_SUCCEEDED(rv));
-      NS_ENSURE_SUCCESS(rv, rv);
-    } else if (offset == node->Length()) {
-      EditorRawDOMPoint afterNode(node);
-      if (NS_WARN_IF(!afterNode.AdvanceOffset())) {
-        return NS_ERROR_FAILURE;
-      }
-      nsresult rv = selection->Collapse(afterNode);
-      MOZ_ASSERT(NS_SUCCEEDED(rv));
-      NS_ENSURE_SUCCESS(rv, rv);
-    } else {
-      nsCOMPtr<nsIDOMNode> tmp;
-      nsresult rv = SplitNode(node->AsDOMNode(), offset, getter_AddRefs(tmp));
-      NS_ENSURE_SUCCESS(rv, rv);
-      EditorRawDOMPoint atNode(node);
-      if (NS_WARN_IF(!atNode.IsSetAndValid())) {
-        return NS_ERROR_FAILURE;
-      }
-      rv = selection->Collapse(atNode);
-      MOZ_ASSERT(NS_SUCCEEDED(rv));
-      NS_ENSURE_SUCCESS(rv, rv);
+  // XXX We want Selection::AnchorRef()
+  uint32_t offset = selection->AnchorOffset();
+
+  if (!offset) {
+    EditorRawDOMPoint atNode(node);
+    if (NS_WARN_IF(!atNode.IsSetAndValid())) {
+      return NS_ERROR_FAILURE;
     }
+    ErrorResult error;
+    selection->Collapse(atNode, error);
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
+    return NS_OK;
+  }
+
+  if (offset == node->Length()) {
+    EditorRawDOMPoint afterNode(node);
+    if (NS_WARN_IF(!afterNode.AdvanceOffset())) {
+      return NS_ERROR_FAILURE;
+    }
+    ErrorResult error;
+    selection->Collapse(afterNode, error);
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
+    return NS_OK;
+  }
+
+  EditorRawDOMPoint atStartOfRightNode(node, offset);
+  MOZ_ASSERT(atStartOfRightNode.IsSetAndValid());
+  ErrorResult error;
+  nsCOMPtr<nsIContent> newLeftNode = SplitNode(atStartOfRightNode, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+
+  EditorRawDOMPoint atRightNode(atStartOfRightNode.Container());
+  if (NS_WARN_IF(!atRightNode.IsSet())) {
+    return NS_ERROR_FAILURE;
+  }
+  MOZ_ASSERT(atRightNode.IsSetAndValid());
+  selection->Collapse(atRightNode, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
   }
   return NS_OK;
 }

editor/libeditor/EditorBase.h

@@ -122,6 +122,7 @@ class InsertTextTransaction;
 class JoinNodeTransaction;
 class PlaceholderTransaction;
 class RemoveStyleSheetTransaction;
+class SplitNodeResult;
 class SplitNodeTransaction;
 class TextComposition;
 class TextEditor;
@@ -209,6 +210,22 @@ private:
 #define kMOZEditorBogusNodeAttrAtom nsGkAtoms::mozeditorbogusnode
 #define kMOZEditorBogusNodeValue NS_LITERAL_STRING("TRUE")
 
+/**
+ * SplitAtEdges is for EditorBase::SplitNodeDeep(),
+ * HTMLEditor::InsertNodeAtPoint()
+ */
+enum class SplitAtEdges
+{
+  // EditorBase::SplitNodeDeep() won't split container element nodes at
+  // their edges.  I.e., when split point is start or end of container,
+  // it won't be split.
+  eDoNotCreateEmptyContainer,
+  // EditorBase::SplitNodeDeep() always splits containers even if the split
+  // point is at edge of a container.  E.g., if split point is start of an
+  // inline element, empty inline element is created as a new left node.
+  eAllowToCreateEmptyContainer
+};
+
 /**
  * Implementation of an editor object.  it will be the controller/focal point
  * for the main editor services. i.e. the GUIManager, publishing, transaction
@@ -345,8 +362,23 @@ public:
                                                  nsAtom* aAttribute = nullptr,
                                                  const nsAString* aValue =
                                                  nullptr);
-  nsIContent* SplitNode(nsIContent& aNode, int32_t aOffset,
-                        ErrorResult& aResult);
+
+  /**
+   * SplitNode() creates a transaction to create a new node (left node)
+   * identical to an existing node (right node), and split the contents
+   * between the same point in both nodes, then, execute the transaction.
+   *
+   * @param aStartOfRightNode   The point to split.  Its container will be
+   *                            the right node, i.e., become the new node's
+   *                            next sibling.  And the point will be start
+   *                            of the right node.
+   * @param aError              If succeed, returns no error.  Otherwise, an
+   *                            error.
+   */
+  already_AddRefed<nsIContent>
+  SplitNode(const EditorRawDOMPoint& aStartOfRightNode,
+            ErrorResult& aResult);
+
   nsresult JoinNodes(nsINode& aLeftNode, nsINode& aRightNode);
   nsresult MoveNode(nsIContent* aNode, nsINode* aParent, int32_t aOffset);
 
@@ -447,7 +479,7 @@ protected:
    * @return                The created new element node.
    */
   already_AddRefed<Element> CreateNode(nsAtom* aTag,
-                                       EditorRawDOMPoint& aPointToInsert);
+                                       const EditorRawDOMPoint& aPointToInsert);
 
   /**
    * Create a transaction for inserting aNode as a child of aParent.
@@ -538,8 +570,20 @@ protected:
     CreateTxnForDeleteCharacter(nsGenericDOMDataNode& aData, uint32_t aOffset,
                                 EDirection aDirection);
 
+  /**
+   * CreateTxnForSplitNode() creates a transaction to create a new node
+   * (left node) identical to an existing node (right node), and split the
+   * contents between the same point in both nodes.
+   *
+   * @param aStartOfRightNode   The point to split.  Its container will be
+   *                            the right node, i.e., become the new node's
+   *                            next sibling.  And the point will be start
+   *                            of the right node.
+   * @return                    The new transaction to split the container of
+   *                            aStartOfRightNode.
+   */
   already_AddRefed<SplitNodeTransaction>
-    CreateTxnForSplitNode(nsIContent& aNode, uint32_t aOffset);
+    CreateTxnForSplitNode(const EditorRawDOMPoint& aStartOfRightNode);
 
   already_AddRefed<JoinNodeTransaction>
     CreateTxnForJoinNode(nsINode& aLeftNode, nsINode& aRightNode);
@@ -731,18 +775,27 @@ public:
   void StopPreservingSelection();
 
   /**
-   * SplitNode() creates a new node identical to an existing node, and split
-   * the contents between the two nodes
-   * @param aExistingRightNode  The node to split.  It will become the new
-   *                            node's next sibling.
-   * @param aOffset             The offset of aExistingRightNode's
-   *                            content|children to do the split at
-   * @param aNewLeftNode        The new node resulting from the split, becomes
-   *                            aExistingRightNode's previous sibling.
+   * SplitNodeImpl() creates a new node (left node) identical to an existing
+   * node (right node), and split the contents between the same point in both
+   * nodes.
+   *
+   * @param aStartOfRightNode   The point to split.  Its container will be
+   *                            the right node, i.e., become the new node's
+   *                            next sibling.  And the point will be start
+   *                            of the right node.
+   * @param aNewLeftNode        The new node called as left node, so, this
+   *                            becomes the container of aPointToSplit's
+   *                            previous sibling.
+   * @param aError              Must have not already failed.
+   *                            If succeed to insert aLeftNode before the
+   *                            right node and remove unnecessary contents
+   *                            (and collapse selection at end of the left
+   *                            node if necessary), returns no error.
+   *                            Otherwise, an error.
    */
-  nsresult SplitNodeImpl(nsIContent& aExistingRightNode,
-                         int32_t aOffset,
-                         nsIContent& aNewLeftNode);
+  void SplitNodeImpl(const EditorDOMPoint& aStartOfRightNode,
+                     nsIContent& aNewLeftNode,
+                     ErrorResult& aError);
 
   /**
    * JoinNodes() takes 2 nodes and merge their content|children.
@@ -1094,15 +1147,28 @@ public:
 
   nsresult IsPreformatted(nsIDOMNode* aNode, bool* aResult);
 
-  enum class EmptyContainers { no, yes };
-  int32_t SplitNodeDeep(nsIContent& aNode, nsIContent& aSplitPointParent,
-                        int32_t aSplitPointOffset,
-                        EmptyContainers aEmptyContainers =
-                          EmptyContainers::yes,
-                        nsIContent** outLeftNode = nullptr,
-                        nsIContent** outRightNode = nullptr,
-                        nsCOMPtr<nsIContent>* ioChildAtSplitPointOffset =
-                          nullptr);
+  /**
+   * SplitNodeDeep() splits aMostAncestorToSplit deeply.
+   *
+   * @param aMostAncestorToSplit        The most ancestor node which should be
+   *                                    split.
+   * @param aStartOfDeepestRightNode    The start point of deepest right node.
+   *                                    This point must be descendant of
+   *                                    aMostAncestorToSplit.
+   * @param aSplitAtEdges               Whether the caller allows this to
+   *                                    create empty container element when
+   *                                    split point is start or end of an
+   *                                    element.
+   * @return                            SplitPoint() returns split point in
+   *                                    aMostAncestorToSplit.  The point must
+   *                                    be good to insert something if the
+   *                                    caller want to do it.
+   */
+  SplitNodeResult
+  SplitNodeDeep(nsIContent& aMostAncestorToSplit,
+                const EditorRawDOMPoint& aDeepestStartOfRightNode,
+                SplitAtEdges aSplitAtEdges);
+
   EditorDOMPoint JoinNodeDeep(nsIContent& aLeftNode,
                               nsIContent& aRightNode);
 

editor/libeditor/EditorDOMPoint.h

@@ -120,6 +120,102 @@ private:
   }
 };
 
+/**
+ * AutoEditorDOMPointOffsetInvalidator is useful if DOM tree will be changed
+ * when EditorDOMPoint instance is available and keeps referring same child
+ * node.
+ *
+ * This class automatically guarantees that given EditorDOMPoint instance
+ * stores the child node and invalidates its offset when the instance is
+ * destroyed.  Additionally, users of this class can invalidate the offset
+ * manually when they need.
+ */
+class MOZ_STACK_CLASS AutoEditorDOMPointOffsetInvalidator final
+{
+public:
+  explicit AutoEditorDOMPointOffsetInvalidator(EditorDOMPoint& aPoint)
+    : mPoint(aPoint)
+  {
+    MOZ_ASSERT(aPoint.IsSetAndValid());
+    mChild = mPoint.GetChildAtOffset();
+  }
+
+  ~AutoEditorDOMPointOffsetInvalidator()
+  {
+    InvalidateOffset();
+  }
+
+  /**
+   * Manually, invalidate offset of the given point.
+   */
+  void InvalidateOffset()
+  {
+    if (mChild) {
+      mPoint.Set(mChild);
+    } else {
+      // If the point referred after the last child, let's keep referring
+      // after current last node of the old container.
+      mPoint.Set(mPoint.Container(), mPoint.Container()->Length());
+    }
+  }
+
+private:
+  EditorDOMPoint& mPoint;
+  // Needs to store child node by ourselves because EditorDOMPoint stores
+  // child node with mRef which is previous sibling of current child node.
+  // Therefore, we cannot keep referring it if it's first child.
+  nsCOMPtr<nsIContent> mChild;
+
+  AutoEditorDOMPointOffsetInvalidator() = delete;
+  AutoEditorDOMPointOffsetInvalidator(
+    const AutoEditorDOMPointOffsetInvalidator& aOther) = delete;
+  const AutoEditorDOMPointOffsetInvalidator& operator=(
+    const AutoEditorDOMPointOffsetInvalidator& aOther) = delete;
+};
+
+/**
+ * AutoEditorDOMPointChildInvalidator is useful if DOM tree will be changed
+ * when EditorDOMPoint instance is available and keeps referring same container
+ * and offset in it.
+ *
+ * This class automatically guarantees that given EditorDOMPoint instance
+ * stores offset and invalidates its child node when the instance is destroyed.
+ * Additionally, users of this class can invalidate the child manually when
+ * they need.
+ */
+class MOZ_STACK_CLASS AutoEditorDOMPointChildInvalidator final
+{
+public:
+  explicit AutoEditorDOMPointChildInvalidator(EditorDOMPoint& aPoint)
+    : mPoint(aPoint)
+  {
+    MOZ_ASSERT(aPoint.IsSetAndValid());
+    Unused << mPoint.Offset();
+  }
+
+  ~AutoEditorDOMPointChildInvalidator()
+  {
+    InvalidateChild();
+  }
+
+  /**
+   * Manually, invalidate child of the given point.
+   */
+  void InvalidateChild()
+  {
+    mPoint.Set(mPoint.Container(), mPoint.Offset());
+  }
+
+private:
+  EditorDOMPoint& mPoint;
+
+  AutoEditorDOMPointChildInvalidator() = delete;
+  AutoEditorDOMPointChildInvalidator(
+    const AutoEditorDOMPointChildInvalidator& aOther) = delete;
+  const AutoEditorDOMPointChildInvalidator& operator=(
+    const AutoEditorDOMPointChildInvalidator& aOther) = delete;
+};
+
 } // namespace mozilla
 
 #endif // #ifndef mozilla_EditorDOMPoint_h

editor/libeditor/EditorUtils.h

@@ -141,6 +141,158 @@ EditActionCanceled(nsresult aRv = NS_OK)
   return EditActionResult(aRv, true, true);
 }
 
+/***************************************************************************
+ * SplitNodeResult is a simple class for EditorBase::SplitNodeDeep().
+ * This makes the callers' code easier to read.
+ */
+class MOZ_STACK_CLASS SplitNodeResult final
+{
+public:
+  bool Succeeded() const { return NS_SUCCEEDED(mRv); }
+  bool Failed() const { return NS_FAILED(mRv); }
+  nsresult Rv() const { return mRv; }
+
+  /**
+   * DidSplit() returns true if a node was actually split.
+   */
+  bool DidSplit() const
+  {
+    return mPreviousNode && mNextNode;
+  }
+
+  /**
+   * GetLeftNode() simply returns the left node which was created at splitting.
+   * This returns nullptr if the node wasn't split.
+   */
+  nsIContent* GetLeftNode() const
+  {
+    return mPreviousNode && mNextNode ? mPreviousNode.get() : nullptr;
+  }
+
+  /**
+   * GetRightNode() simply returns the right node which was split.
+   * This won't return nullptr unless failed to split due to invalid arguments.
+   */
+  nsIContent* GetRightNode() const
+  {
+    if (mGivenSplitPoint.IsSet()) {
+      return mGivenSplitPoint.GetChildAtOffset();
+    }
+    return mPreviousNode && !mNextNode ? mPreviousNode : mNextNode;
+  }
+
+  /**
+   * GetPreviousNode() returns previous node at the split point.
+   */
+  nsIContent* GetPreviousNode() const
+  {
+    if (mGivenSplitPoint.IsSet()) {
+      return mGivenSplitPoint.IsEndOfContainer() ?
+               mGivenSplitPoint.GetChildAtOffset() : nullptr;
+    }
+    return mPreviousNode;
+  }
+
+  /**
+   * GetNextNode() returns next node at the split point.
+   */
+  nsIContent* GetNextNode() const
+  {
+    if (mGivenSplitPoint.IsSet()) {
+      return !mGivenSplitPoint.IsEndOfContainer() ?
+                mGivenSplitPoint.GetChildAtOffset() : nullptr;
+    }
+    return mNextNode;
+  }
+
+  /**
+   * SplitPoint() returns the split point in the container.
+   * This is useful when callers insert an element at split point with
+   * EditorBase::CreateNode() or something similar methods.
+   *
+   * Note that the result is EditorRawDOMPoint but the nodes are grabbed
+   * by this instance.  Therefore, the life time of both container node
+   * and child node are guaranteed while using the result temporarily.
+   */
+  EditorRawDOMPoint SplitPoint() const
+  {
+    if (Failed()) {
+      return EditorRawDOMPoint();
+    }
+    if (mGivenSplitPoint.IsSet()) {
+      return mGivenSplitPoint.AsRaw();
+    }
+    if (!mPreviousNode) {
+      return EditorRawDOMPoint(mNextNode);
+    }
+    EditorRawDOMPoint point(mPreviousNode);
+    DebugOnly<bool> advanced = point.AdvanceOffset();
+    NS_WARNING_ASSERTION(advanced,
+      "Failed to advance offset to after previous node");
+    return point;
+  }
+
+  /**
+   * This constructor shouldn't be used by anybody except methods which
+   * use this as result when it succeeds.
+   *
+   * @param aPreviousNodeOfSplitPoint   Previous node immediately before
+   *                                    split point.
+   * @param aNextNodeOfSplitPoint       Next node immediately after split
+   *                                    point.
+   */
+  SplitNodeResult(nsIContent* aPreviousNodeOfSplitPoint,
+                  nsIContent* aNextNodeOfSplitPoint)
+    : mPreviousNode(aPreviousNodeOfSplitPoint)
+    , mNextNode(aNextNodeOfSplitPoint)
+    , mRv(NS_OK)
+  {
+    MOZ_DIAGNOSTIC_ASSERT(mPreviousNode || mNextNode);
+  }
+
+  /**
+   * This constructor should be used when the method didn't split any nodes
+   * but want to return given split point as right point.
+   */
+  explicit SplitNodeResult(const EditorRawDOMPoint& aGivenSplitPoint)
+    : mGivenSplitPoint(aGivenSplitPoint)
+    , mRv(NS_OK)
+  {
+    MOZ_DIAGNOSTIC_ASSERT(mGivenSplitPoint.IsSet());
+  }
+
+  /**
+   * This constructor shouldn't be used by anybody except methods which
+   * use this as error result when it fails.
+   */
+  explicit SplitNodeResult(nsresult aRv)
+    : mRv(aRv)
+  {
+    MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(mRv));
+  }
+
+private:
+  // When methods which return this class split some nodes actually, they
+  // need to set a set of left node and right node to this class.  However,
+  // one or both of them may be moved or removed by mutation observer.
+  // In such case, we cannot represent the point with EditorDOMPoint since
+  // it requires current container node.  Therefore, we need to use
+  // nsCOMPtr<nsIContent> here instead.
+  nsCOMPtr<nsIContent> mPreviousNode;
+  nsCOMPtr<nsIContent> mNextNode;
+
+  // Methods which return this class may not split any nodes actually.  Then,
+  // they may want to return given split point as is since such behavior makes
+  // their callers simpler.  In this case, the point may be in a text node
+  // which cannot be represented as a node.  Therefore, we need EditorDOMPoint
+  // for representing the point.
+  EditorDOMPoint mGivenSplitPoint;
+
+  nsresult mRv;
+
+  SplitNodeResult() = delete;
+};
+
 /***************************************************************************
  * stack based helper class for batching a collection of transactions inside a
  * placeholder transaction.

editor/libeditor/HTMLEditRules.h

@@ -32,6 +32,7 @@ namespace mozilla {
 class EditActionResult;
 class HTMLEditor;
 class RulesInfo;
+class SplitNodeResult;
 class TextEditor;
 namespace dom {
 class Element;
@@ -118,8 +119,8 @@ public:
   NS_IMETHOD DidDeleteNode(nsIDOMNode* aChild, nsresult aResult) override;
   NS_IMETHOD WillSplitNode(nsIDOMNode* aExistingRightNode,
                            int32_t aOffset) override;
-  NS_IMETHOD DidSplitNode(nsIDOMNode* aExistingRightNode, int32_t aOffset,
-                          nsIDOMNode* aNewLeftNode, nsresult aResult) override;
+  NS_IMETHOD DidSplitNode(nsIDOMNode* aExistingRightNode,
+                          nsIDOMNode* aNewLeftNode) override;
   NS_IMETHOD WillJoinNodes(nsIDOMNode* aLeftNode, nsIDOMNode* aRightNode,
                            nsIDOMNode* aParent) override;
   NS_IMETHOD DidJoinNodes(nsIDOMNode* aLeftNode, nsIDOMNode* aRightNode,
@@ -314,18 +315,20 @@ protected:
   /**
    * SplitParagraph() splits the parent block, aPara, at aSelNode - aOffset.
    *
-   * @param aSelection  The selection.
-   * @param aPara       The parent block to be split.
-   * @param aBRNode     Next <br> node if there is.  Otherwise, nullptr.
-   *                    If this is not nullptr, the <br> node may be removed.
-   * @param aSelNode    Set the selection container to split aPara at.
-   * @param aOffset     Set the offset in the container.
+   * @param aSelection          The selection.
+   * @param aParentDivOrP       The parent block to be split.  This must be <p>
+   *                            or <div> element.
+   * @param aStartOfRightNode   The point to be start of right node after
+   *                            split.  This must be descendant of
+   *                            aParentDivOrP.
+   * @param aNextBRNode         Next <br> node if there is.  Otherwise, nullptr.
+   *                            If this is not nullptr, the <br> node may be
+   *                            removed.
    */
   nsresult SplitParagraph(Selection& aSelection,
-                          Element& aPara,
-                          nsIContent* aBRNode,
-                          nsINode& aSelNode,
-                          int32_t aOffset);
+                          Element& aParentDivOrP,
+                          const EditorRawDOMPoint& aStartOfRightNode,
+                          nsIContent* aBRNode);
 
   nsresult ReturnInListItem(Selection& aSelection, Element& aHeader,
                             nsINode& aNode, int32_t aOffset);
@@ -413,12 +416,28 @@ protected:
   nsresult ApplyBlockStyle(nsTArray<OwningNonNull<nsINode>>& aNodeArray,
                            nsAtom& aBlockTag);
   nsresult MakeBlockquote(nsTArray<OwningNonNull<nsINode>>& aNodeArray);
-  nsresult SplitAsNeeded(nsAtom& aTag, OwningNonNull<nsINode>& inOutParent,
-                         int32_t& inOutOffset,
-                         nsCOMPtr<nsIContent>* inOutChildAtOffset = nullptr);
-  nsresult SplitAsNeeded(nsAtom& aTag, nsCOMPtr<nsINode>& inOutParent,
-                         int32_t& inOutOffset,
-                         nsCOMPtr<nsIContent>* inOutChildAtOffset = nullptr);
+
+  /**
+   * MaybeSplitAncestorsForInsert() does nothing if container of
+   * aStartOfDeepestRightNode can have an element whose tag name is aTag.
+   * Otherwise, looks for an ancestor node which is or is in active editing
+   * host and can have an element whose name is aTag.  If there is such
+   * ancestor, its descendants are split.
+   *
+   * Note that this may create empty elements while splitting ancestors.
+   *
+   * @param aTag                        The name of element to be inserted
+   *                                    after calling this method.
+   * @param aStartOfDeepestRightNode    The start point of deepest right node.
+   *                                    This point must be descendant of
+   *                                    active editing host.
+   * @return                            When succeeded, SplitPoint() returns
+   *                                    the point to insert the element.
+   */
+  SplitNodeResult MaybeSplitAncestorsForInsert(
+                    nsAtom& aTag,
+                    const EditorRawDOMPoint& aStartOfDeepestRightNode);
+
   nsresult AddTerminatingBR(nsIDOMNode *aBlock);
   EditorDOMPoint JoinNodesSmart(nsIContent& aNodeLeft,
                                 nsIContent& aNodeRight);

editor/libeditor/HTMLEditor.cpp

@@ -1522,7 +1522,8 @@ HTMLEditor::InsertElementAtSelection(nsIDOMElement* aElement,
                                  &offsetForInsert);
 
       rv = InsertNodeAtPoint(node, address_of(parentSelectedDOMNode),
-                             &offsetForInsert, false);
+                             &offsetForInsert,
+                             SplitAtEdges::eAllowToCreateEmptyContainer);
       NS_ENSURE_SUCCESS(rv, rv);
       // Set caret after element, but check for special case
       //  of inserting table-related elements: set in first cell instead
@@ -1564,14 +1565,14 @@ HTMLEditor::InsertElementAtSelection(nsIDOMElement* aElement,
  * @param aNode             Node to insert.
  * @param ioParent          Insertion parent.
  * @param ioOffset          Insertion offset.
- * @param aNoEmptyNodes     Splitting can result in empty nodes?
+ * @param aSplitAtEdges     Splitting can result in empty nodes?
  * @param ioChildAtOffset   Child node at insertion offset (optional).
  */
 nsresult
 HTMLEditor::InsertNodeAtPoint(nsIDOMNode* aNode,
                               nsCOMPtr<nsIDOMNode>* ioParent,
                               int32_t* ioOffset,
-                              bool aNoEmptyNodes,
+                              SplitAtEdges aSplitAtEdges,
                               nsCOMPtr<nsIDOMNode>* ioChildAtOffset)
 {
   nsCOMPtr<nsIContent> node = do_QueryInterface(aNode);
@@ -1618,20 +1619,18 @@ HTMLEditor::InsertNodeAtPoint(nsIDOMNode* aNode,
     parent = parent->GetParent();
   }
   if (parent != topChild) {
-    nsCOMPtr<nsIContent> child;
-    if (ioChildAtOffset) {
-      child = do_QueryInterface(*ioChildAtOffset);
+    // We need to split some levels above the original selection parent.
+    SplitNodeResult splitNodeResult =
+      SplitNodeDeep(*topChild, EditorRawDOMPoint(origParent, *ioOffset),
+                    aSplitAtEdges);
+    if (NS_WARN_IF(splitNodeResult.Failed())) {
+      return splitNodeResult.Rv();
     }
-    // we need to split some levels above the original selection parent
-    int32_t offset = SplitNodeDeep(*topChild, *origParent, *ioOffset,
-                                   aNoEmptyNodes ? EmptyContainers::no
-                                                 : EmptyContainers::yes,
-                                   nullptr, nullptr, address_of(child));
-    NS_ENSURE_STATE(offset != -1);
-    *ioParent = GetAsDOMNode(parent);
-    *ioOffset = offset;
+    EditorRawDOMPoint splitPoint(splitNodeResult.SplitPoint());
+    *ioParent = GetAsDOMNode(splitPoint.Container());
+    *ioOffset = splitPoint.Offset();
     if (ioChildAtOffset) {
-      *ioChildAtOffset = GetAsDOMNode(child);
+      *ioChildAtOffset = GetAsDOMNode(splitPoint.GetChildAtOffset());
     }
   }
   // Now we can insert the new node
@@ -1975,51 +1974,56 @@ HTMLEditor::MakeOrChangeList(const nsAString& aListType,
     return rv;
   }
 
-  if (!handled) {
-    // Find out if the selection is collapsed:
-    bool isCollapsed = selection->Collapsed();
-
-    NS_ENSURE_TRUE(selection->GetRangeAt(0) &&
-                   selection->GetRangeAt(0)->GetStartContainer() &&
-                   selection->GetRangeAt(0)->GetStartContainer()->IsContent(),
-                   NS_ERROR_FAILURE);
-    OwningNonNull<nsIContent> node =
-      *selection->GetRangeAt(0)->GetStartContainer()->AsContent();
-    int32_t offset = selection->GetRangeAt(0)->StartOffset();
-    nsCOMPtr<nsIContent> child =
-      selection->GetRangeAt(0)->GetChildAtStartOffset();
-
-    if (isCollapsed) {
-      // have to find a place to put the list
-      nsCOMPtr<nsIContent> parent = node;
-      nsCOMPtr<nsIContent> topChild = node;
-
-      RefPtr<nsAtom> listAtom = NS_Atomize(aListType);
-      while (!CanContainTag(*parent, *listAtom)) {
-        topChild = parent;
-        parent = parent->GetParent();
-      }
-
-      if (parent != node) {
-        // we need to split up to the child of parent
-        offset = SplitNodeDeep(*topChild, *node, offset,
-                               EmptyContainers::yes, nullptr, nullptr,
-                               address_of(child));
-        NS_ENSURE_STATE(offset != -1);
-      }
-
-      // make a list
-      MOZ_DIAGNOSTIC_ASSERT(child);
-      EditorRawDOMPoint atChild(parent, child, offset);
-      RefPtr<Element> newList = CreateNode(listAtom, atChild);
-      NS_ENSURE_STATE(newList);
-      // make a list item
-      EditorRawDOMPoint atStartOfNewList(newList, 0);
-      RefPtr<Element> newItem = CreateNode(nsGkAtoms::li, atStartOfNewList);
-      NS_ENSURE_STATE(newItem);
-      rv = selection->Collapse(newItem, 0);
-      NS_ENSURE_SUCCESS(rv, rv);
+  if (!handled && selection->Collapsed()) {
+    nsRange* firstRange = selection->GetRangeAt(0);
+    if (NS_WARN_IF(!firstRange)) {
+      return NS_ERROR_FAILURE;
     }
+
+    EditorRawDOMPoint atStartOfSelection(firstRange->StartRef());
+    if (NS_WARN_IF(!atStartOfSelection.IsSet()) ||
+        NS_WARN_IF(!atStartOfSelection.Container()->IsContent())) {
+      return NS_ERROR_FAILURE;
+    }
+
+    // Have to find a place to put the list.
+    EditorDOMPoint pointToInsertList(atStartOfSelection);
+
+    RefPtr<nsAtom> listAtom = NS_Atomize(aListType);
+    while (!CanContainTag(*pointToInsertList.Container(), *listAtom)) {
+      pointToInsertList.Set(pointToInsertList.Container());
+      if (NS_WARN_IF(!pointToInsertList.IsSet()) ||
+          NS_WARN_IF(!pointToInsertList.Container()->IsContent())) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
+    if (pointToInsertList.Container() != atStartOfSelection.Container()) {
+      // We need to split up to the child of parent.
+      SplitNodeResult splitNodeResult =
+        SplitNodeDeep(*pointToInsertList.GetChildAtOffset(),
+                      atStartOfSelection,
+                      SplitAtEdges::eAllowToCreateEmptyContainer);
+      if (NS_WARN_IF(splitNodeResult.Failed())) {
+        return splitNodeResult.Rv();
+      }
+      pointToInsertList = splitNodeResult.SplitPoint();
+      if (NS_WARN_IF(!pointToInsertList.IsSet())) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
+    // Create a list and insert it before the right node if we split some
+    // parents of start of selection above, or just start of selection
+    // otherwise.
+    RefPtr<Element> newList = CreateNode(listAtom, pointToInsertList.AsRaw());
+    NS_ENSURE_STATE(newList);
+    // make a list item
+    EditorRawDOMPoint atStartOfNewList(newList, 0);
+    RefPtr<Element> newItem = CreateNode(nsGkAtoms::li, atStartOfNewList);
+    NS_ENSURE_STATE(newItem);
+    rv = selection->Collapse(newItem, 0);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return rules->DidDoAction(selection, &ruleInfo, rv);
@@ -2119,50 +2123,55 @@ HTMLEditor::InsertBasicBlock(const nsAString& aBlockType)
     return rv;
   }
 
-  if (!handled) {
-    // Find out if the selection is collapsed:
-    bool isCollapsed = selection->Collapsed();
-
-    NS_ENSURE_TRUE(selection->GetRangeAt(0) &&
-                   selection->GetRangeAt(0)->GetStartContainer() &&
-                   selection->GetRangeAt(0)->GetStartContainer()->IsContent(),
-                   NS_ERROR_FAILURE);
-    OwningNonNull<nsIContent> node =
-      *selection->GetRangeAt(0)->GetStartContainer()->AsContent();
-    int32_t offset = selection->GetRangeAt(0)->StartOffset();
-    nsCOMPtr<nsIContent> child =
-      selection->GetRangeAt(0)->GetChildAtStartOffset();
-
-    if (isCollapsed) {
-      // have to find a place to put the block
-      nsCOMPtr<nsIContent> parent = node;
-      nsCOMPtr<nsIContent> topChild = node;
-
-      RefPtr<nsAtom> blockAtom = NS_Atomize(aBlockType);
-      while (!CanContainTag(*parent, *blockAtom)) {
-        NS_ENSURE_TRUE(parent->GetParent(), NS_ERROR_FAILURE);
-        topChild = parent;
-        parent = parent->GetParent();
-      }
-
-      if (parent != node) {
-        // we need to split up to the child of parent
-        offset = SplitNodeDeep(*topChild, *node, offset,
-                               EmptyContainers::yes, nullptr, nullptr,
-                               address_of(child));
-        NS_ENSURE_STATE(offset != -1);
-      }
-
-      // make a block
-      MOZ_DIAGNOSTIC_ASSERT(child);
-      EditorRawDOMPoint atChild(parent, child, offset);
-      RefPtr<Element> newBlock = CreateNode(blockAtom, atChild);
-      NS_ENSURE_STATE(newBlock);
-
-      // reposition selection to inside the block
-      rv = selection->Collapse(newBlock, 0);
-      NS_ENSURE_SUCCESS(rv, rv);
+  if (!handled && selection->Collapsed()) {
+    nsRange* firstRange = selection->GetRangeAt(0);
+    if (NS_WARN_IF(!firstRange)) {
+      return NS_ERROR_FAILURE;
     }
+
+    EditorRawDOMPoint atStartOfSelection(firstRange->StartRef());
+    if (NS_WARN_IF(!atStartOfSelection.IsSet()) ||
+        NS_WARN_IF(!atStartOfSelection.Container()->IsContent())) {
+      return NS_ERROR_FAILURE;
+    }
+
+    // Have to find a place to put the block.
+    EditorDOMPoint pointToInsertBlock(atStartOfSelection);
+
+    RefPtr<nsAtom> blockAtom = NS_Atomize(aBlockType);
+    while (!CanContainTag(*pointToInsertBlock.Container(), *blockAtom)) {
+      pointToInsertBlock.Set(pointToInsertBlock.Container());
+      if (NS_WARN_IF(!pointToInsertBlock.IsSet()) ||
+          NS_WARN_IF(!pointToInsertBlock.Container()->IsContent())) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
+    if (pointToInsertBlock.Container() != atStartOfSelection.Container()) {
+      // We need to split up to the child of the point to insert a block.
+      SplitNodeResult splitBlockResult =
+        SplitNodeDeep(*pointToInsertBlock.GetChildAtOffset(),
+                      atStartOfSelection,
+                      SplitAtEdges::eAllowToCreateEmptyContainer);
+      if (NS_WARN_IF(splitBlockResult.Failed())) {
+        return splitBlockResult.Rv();
+      }
+      pointToInsertBlock = splitBlockResult.SplitPoint();
+      if (NS_WARN_IF(!pointToInsertBlock.IsSet())) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
+    // Create a block and insert it before the right node if we split some
+    // parents of start of selection above, or just start of selection
+    // otherwise.
+    RefPtr<Element> newBlock =
+      CreateNode(blockAtom, pointToInsertBlock.AsRaw());
+    NS_ENSURE_STATE(newBlock);
+
+    // reposition selection to inside the block
+    rv = selection->Collapse(newBlock, 0);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return rules->DidDoAction(selection, &ruleInfo, rv);
@@ -2196,56 +2205,62 @@ HTMLEditor::Indent(const nsAString& aIndent)
     return rv;
   }
 
-  if (!handled) {
-    // Do default - insert a blockquote node if selection collapsed
-    bool isCollapsed = selection->Collapsed();
+  if (!handled && selection->Collapsed() && aIndent.EqualsLiteral("indent")) {
+    nsRange* firstRange = selection->GetRangeAt(0);
+    if (NS_WARN_IF(!firstRange)) {
+      return NS_ERROR_FAILURE;
+    }
 
-    NS_ENSURE_TRUE(selection->GetRangeAt(0) &&
-                   selection->GetRangeAt(0)->GetStartContainer() &&
-                   selection->GetRangeAt(0)->GetStartContainer()->IsContent(),
-                   NS_ERROR_FAILURE);
-    OwningNonNull<nsIContent> node =
-      *selection->GetRangeAt(0)->GetStartContainer()->AsContent();
-    int32_t offset = selection->GetRangeAt(0)->StartOffset();
-    nsCOMPtr<nsIContent> child =
-      selection->GetRangeAt(0)->GetChildAtStartOffset();
+    EditorRawDOMPoint atStartOfSelection(firstRange->StartRef());
+    if (NS_WARN_IF(!atStartOfSelection.IsSet()) ||
+        NS_WARN_IF(!atStartOfSelection.Container()->IsContent())) {
+      return NS_ERROR_FAILURE;
+    }
 
-    if (aIndent.EqualsLiteral("indent")) {
-      if (isCollapsed) {
-        // have to find a place to put the blockquote
-        nsCOMPtr<nsIContent> parent = node;
-        nsCOMPtr<nsIContent> topChild = node;
-        while (!CanContainTag(*parent, *nsGkAtoms::blockquote)) {
-          NS_ENSURE_TRUE(parent->GetParent(), NS_ERROR_FAILURE);
-          topChild = parent;
-          parent = parent->GetParent();
-        }
+    // Have to find a place to put the blockquote.
+    EditorDOMPoint pointToInsertBlockquote(atStartOfSelection);
 
-        if (parent != node) {
-          // we need to split up to the child of parent
-          offset = SplitNodeDeep(*topChild, *node, offset,
-                                 EmptyContainers::yes, nullptr, nullptr,
-                                 address_of(child));
-          NS_ENSURE_STATE(offset != -1);
-        }
-
-        // make a blockquote
-        MOZ_DIAGNOSTIC_ASSERT(child);
-        EditorRawDOMPoint atChild(parent, child, offset);
-        RefPtr<Element> newBQ = CreateNode(nsGkAtoms::blockquote, atChild);
-        NS_ENSURE_STATE(newBQ);
-        // put a space in it so layout will draw the list item
-        rv = selection->Collapse(newBQ, 0);
-        NS_ENSURE_SUCCESS(rv, rv);
-        rv = InsertText(NS_LITERAL_STRING(" "));
-        NS_ENSURE_SUCCESS(rv, rv);
-        // reposition selection to before the space character
-        NS_ENSURE_STATE(selection->GetRangeAt(0));
-        rv = selection->Collapse(selection->GetRangeAt(0)->GetStartContainer(),
-                                 0);
-        NS_ENSURE_SUCCESS(rv, rv);
+    while (!CanContainTag(*pointToInsertBlockquote.Container(),
+                          *nsGkAtoms::blockquote)) {
+      pointToInsertBlockquote.Set(pointToInsertBlockquote.Container());
+      if (NS_WARN_IF(!pointToInsertBlockquote.IsSet()) ||
+          NS_WARN_IF(!pointToInsertBlockquote.Container()->IsContent())) {
+        return NS_ERROR_FAILURE;
       }
     }
+
+    if (pointToInsertBlockquote.Container() !=
+          atStartOfSelection.Container()) {
+      // We need to split up to the child of parent.
+      SplitNodeResult splitBlockquoteResult =
+        SplitNodeDeep(*pointToInsertBlockquote.GetChildAtOffset(),
+                      atStartOfSelection,
+                      SplitAtEdges::eAllowToCreateEmptyContainer);
+      if (NS_WARN_IF(splitBlockquoteResult.Failed())) {
+        return splitBlockquoteResult.Rv();
+      }
+      pointToInsertBlockquote = splitBlockquoteResult.SplitPoint();
+      if (NS_WARN_IF(!pointToInsertBlockquote.IsSet())) {
+        return NS_ERROR_FAILURE;
+      }
+    }
+
+    // Create a list and insert it before the right node if we split some
+    // parents of start of selection above, or just start of selection
+    // otherwise.
+    RefPtr<Element> newBQ =
+      CreateNode(nsGkAtoms::blockquote, pointToInsertBlockquote.AsRaw());
+    NS_ENSURE_STATE(newBQ);
+    // put a space in it so layout will draw the list item
+    rv = selection->Collapse(newBQ, 0);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = InsertText(NS_LITERAL_STRING(" "));
+    NS_ENSURE_SUCCESS(rv, rv);
+    // reposition selection to before the space character
+    NS_ENSURE_STATE(selection->GetRangeAt(0));
+    rv = selection->Collapse(selection->GetRangeAt(0)->GetStartContainer(),
+                             0);
+    NS_ENSURE_SUCCESS(rv, rv);
   }
   return rules->DidDoAction(selection, &ruleInfo, rv);
 }

editor/libeditor/HTMLEditor.h

@@ -341,7 +341,7 @@ public:
   nsresult InsertNodeAtPoint(nsIDOMNode* aNode,
                              nsCOMPtr<nsIDOMNode>* ioParent,
                              int32_t* ioOffset,
-                             bool aNoEmptyNodes,
+                             SplitAtEdges aSplitAtEdges,
                              nsCOMPtr<nsIDOMNode>* ioChildAtOffset = nullptr);
 
   /**

editor/libeditor/HTMLEditorDataTransfer.cpp

@@ -272,10 +272,6 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
   }
 
   // Are there any table elements in the list?
-  // node and offset for insertion
-  nsCOMPtr<nsIDOMNode> parentNode;
-  int32_t offsetOfNewNode;
-
   // check for table cell selection mode
   bool cellSelectionMode = false;
   nsCOMPtr<nsIDOMElement> cell;
@@ -332,6 +328,8 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
   if (!handled) {
     // The rules code (WillDoAction above) might have changed the selection.
     // refresh our memory...
+    nsCOMPtr<nsIDOMNode> parentNode;
+    int32_t offsetOfNewNode;
     rv = GetStartNodeAndOffset(selection, getter_AddRefs(parentNode), &offsetOfNewNode);
     NS_ENSURE_SUCCESS(rv, rv);
     NS_ENSURE_TRUE(parentNode, NS_ERROR_FAILURE);
@@ -357,14 +355,22 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
     // Are we in a text node? If so, split it.
     if (IsTextNode(parentNode)) {
       nsCOMPtr<nsIContent> parentContent = do_QueryInterface(parentNode);
-      NS_ENSURE_STATE(parentContent || !parentNode);
-      offsetOfNewNode = SplitNodeDeep(*parentContent, *parentContent,
-                                      offsetOfNewNode);
-      NS_ENSURE_STATE(offsetOfNewNode != -1);
-      nsCOMPtr<nsIDOMNode> temp;
-      rv = parentNode->GetParentNode(getter_AddRefs(temp));
-      NS_ENSURE_SUCCESS(rv, rv);
-      parentNode = temp;
+      EditorRawDOMPoint pointToSplit(parentContent, offsetOfNewNode);
+      if (NS_WARN_IF(!pointToSplit.IsSet())) {
+        return NS_ERROR_FAILURE;
+      }
+      SplitNodeResult splitNodeResult =
+        SplitNodeDeep(*parentContent, pointToSplit,
+                      SplitAtEdges::eAllowToCreateEmptyContainer);
+      if (NS_WARN_IF(splitNodeResult.Failed())) {
+        return splitNodeResult.Rv();
+      }
+      EditorRawDOMPoint splitPoint(splitNodeResult.SplitPoint());
+      if (NS_WARN_IF(!splitPoint.IsSet())) {
+        return NS_ERROR_FAILURE;
+      }
+      parentNode = do_QueryInterface(splitPoint.Container());
+      offsetOfNewNode = splitPoint.Offset();
     }
 
     // build up list of parents of first node in list that are either
@@ -446,7 +452,9 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
         nsCOMPtr<nsIDOMNode> child;
         curNode->GetFirstChild(getter_AddRefs(child));
         while (child) {
-          rv = InsertNodeAtPoint(child, address_of(parentNode), &offsetOfNewNode, true);
+          rv = InsertNodeAtPoint(child, address_of(parentNode),
+                                 &offsetOfNewNode,
+                                 SplitAtEdges::eDoNotCreateEmptyContainer);
           if (NS_FAILED(rv)) {
             break;
           }
@@ -485,7 +493,9 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
                 }
               }
             }
-            rv = InsertNodeAtPoint(child, address_of(parentNode), &offsetOfNewNode, true);
+            rv = InsertNodeAtPoint(child, address_of(parentNode),
+                                   &offsetOfNewNode,
+                                   SplitAtEdges::eDoNotCreateEmptyContainer);
             if (NS_FAILED(rv)) {
               break;
             }
@@ -504,7 +514,9 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
         nsCOMPtr<nsIDOMNode> child;
         curNode->GetFirstChild(getter_AddRefs(child));
         while (child) {
-          rv = InsertNodeAtPoint(child, address_of(parentNode), &offsetOfNewNode, true);
+          rv = InsertNodeAtPoint(child, address_of(parentNode),
+                                 &offsetOfNewNode,
+                                 SplitAtEdges::eDoNotCreateEmptyContainer);
           if (NS_FAILED(rv)) {
             break;
           }
@@ -519,7 +531,9 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
 
       if (!bDidInsert || NS_FAILED(rv)) {
         // try to insert
-        rv = InsertNodeAtPoint(curNode, address_of(parentNode), &offsetOfNewNode, true);
+        rv = InsertNodeAtPoint(curNode, address_of(parentNode),
+                               &offsetOfNewNode,
+                               SplitAtEdges::eDoNotCreateEmptyContainer);
         if (NS_SUCCEEDED(rv)) {
           bDidInsert = true;
           lastInsertNode = curNode;
@@ -531,7 +545,9 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
         while (NS_FAILED(rv) && curNode) {
           curNode->GetParentNode(getter_AddRefs(parent));
           if (parent && !TextEditUtils::IsBody(parent)) {
-            rv = InsertNodeAtPoint(parent, address_of(parentNode), &offsetOfNewNode, true,
+            rv = InsertNodeAtPoint(parent, address_of(parentNode),
+                                   &offsetOfNewNode,
+                                   SplitAtEdges::eDoNotCreateEmptyContainer,
                                    address_of(lastInsertNode));
             if (NS_SUCCEEDED(rv)) {
               bDidInsert = true;
@@ -638,11 +654,13 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString,
         NS_ENSURE_STATE(linkContent || !link);
         nsCOMPtr<nsIContent> selContent = do_QueryInterface(selNode);
         NS_ENSURE_STATE(selContent || !selNode);
-        nsCOMPtr<nsIContent> leftLink;
-        SplitNodeDeep(*linkContent, *selContent, selOffset,
-                      EmptyContainers::no, getter_AddRefs(leftLink));
-        if (leftLink) {
-          EditorRawDOMPoint afterLeftLink(leftLink);
+        SplitNodeResult splitLinkResult =
+          SplitNodeDeep(*linkContent, EditorRawDOMPoint(selContent, selOffset),
+                        SplitAtEdges::eDoNotCreateEmptyContainer);
+        NS_WARNING_ASSERTION(splitLinkResult.Succeeded(),
+          "Failed to split the link");
+        if (splitLinkResult.GetPreviousNode()) {
+          EditorRawDOMPoint afterLeftLink(splitLinkResult.GetPreviousNode());
           if (afterLeftLink.AdvanceOffset()) {
             selection->Collapse(afterLeftLink);
           }

editor/libeditor/HTMLStyleEditor.cpp

@@ -285,37 +285,49 @@ HTMLEditor::SetInlinePropertyOnTextNode(Text& aText,
     return NS_OK;
   }
 
-  // Do we need to split the text node?
-  ErrorResult rv;
-  nsCOMPtr<nsIContent> text = &aText;
-  if (uint32_t(aEndOffset) != aText.Length()) {
+  // Make the range an independent node.
+  nsCOMPtr<nsIContent> textNodeForTheRange = &aText;
+
+  // Split at the end of the range.
+  EditorRawDOMPoint atEnd(textNodeForTheRange, aEndOffset);
+  if (!atEnd.IsEndOfContainer()) {
     // We need to split off back of text node
-    text = SplitNode(*text, aEndOffset, rv);
-    NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
+    ErrorResult error;
+    textNodeForTheRange = SplitNode(atEnd, error);
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
   }
 
-  if (aStartOffset) {
+  // Split at the start of the range.
+  EditorRawDOMPoint atStart(textNodeForTheRange, aStartOffset);
+  if (!atStart.IsStartOfContainer()) {
     // We need to split off front of text node
-    SplitNode(*text, aStartOffset, rv);
-    NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
+    ErrorResult error;
+    nsCOMPtr<nsIContent> newLeftNode = SplitNode(atStart, error);
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
+    Unused << newLeftNode;
   }
 
   if (aAttribute) {
     // Look for siblings that are correct type of node
-    nsIContent* sibling = GetPriorHTMLSibling(text);
+    nsIContent* sibling = GetPriorHTMLSibling(textNodeForTheRange);
     if (IsSimpleModifiableNode(sibling, &aProperty, aAttribute, &aValue)) {
       // Previous sib is already right kind of inline node; slide this over
-      return MoveNode(text, sibling, -1);
+      return MoveNode(textNodeForTheRange, sibling, -1);
     }
-    sibling = GetNextHTMLSibling(text);
+    sibling = GetNextHTMLSibling(textNodeForTheRange);
     if (IsSimpleModifiableNode(sibling, &aProperty, aAttribute, &aValue)) {
       // Following sib is already right kind of inline node; slide this over
-      return MoveNode(text, sibling, 0);
+      return MoveNode(textNodeForTheRange, sibling, 0);
     }
   }
 
   // Reparent the node inside inline node with appropriate {attribute,value}
-  return SetInlinePropertyOnNode(*text, aProperty, aAttribute, aValue);
+  return SetInlinePropertyOnNode(*textNodeForTheRange,
+                                 aProperty, aAttribute, aValue);
 }
 
 nsresult
@@ -513,6 +525,13 @@ HTMLEditor::SplitStyleAbovePoint(nsCOMPtr<nsINode>* aNode,
   NS_ENSURE_TRUE(aNode && *aNode && aOffset, NS_ERROR_NULL_POINTER);
   NS_ENSURE_TRUE((*aNode)->IsContent(), NS_OK);
 
+  if (aOutLeftNode) {
+    *aOutLeftNode = nullptr;
+  }
+  if (aOutRightNode) {
+    *aOutRightNode = nullptr;
+  }
+
   // Split any matching style nodes above the node/offset
   OwningNonNull<nsIContent> node = *(*aNode)->AsContent();
 
@@ -541,13 +560,21 @@ HTMLEditor::SplitStyleAbovePoint(nsCOMPtr<nsINode>* aNode,
         // or the style is specified in the style attribute
         isSet) {
       // Found a style node we need to split
-      int32_t offset = SplitNodeDeep(*node, *(*aNode)->AsContent(), *aOffset,
-                                     EmptyContainers::yes, aOutLeftNode,
-                                     aOutRightNode);
-      NS_ENSURE_TRUE(offset != -1, NS_ERROR_FAILURE);
-      // reset startNode/startOffset
-      *aNode = node->GetParent();
-      *aOffset = offset;
+      SplitNodeResult splitNodeResult =
+        SplitNodeDeep(*node, EditorRawDOMPoint(*aNode, *aOffset),
+                      SplitAtEdges::eAllowToCreateEmptyContainer);
+      NS_WARNING_ASSERTION(splitNodeResult.Succeeded(),
+        "Failed to split the node");
+
+      EditorRawDOMPoint atRightNode(splitNodeResult.SplitPoint());
+      *aNode = atRightNode.Container();
+      *aOffset = atRightNode.Offset();
+      if (aOutLeftNode) {
+        NS_IF_ADDREF(*aOutLeftNode = splitNodeResult.GetPreviousNode());
+      }
+      if (aOutRightNode) {
+        NS_IF_ADDREF(*aOutRightNode = splitNodeResult.GetNextNode());
+      }
     }
     node = node->GetParent();
   }
@@ -1436,47 +1463,58 @@ HTMLEditor::RelativeFontChangeOnTextNode(FontSize aDir,
     return NS_OK;
   }
 
-  OwningNonNull<nsIContent> node = aTextNode;
-
-  // Do we need to split the text node?
-
   // -1 is a magic value meaning to the end of node
   if (aEndOffset == -1) {
     aEndOffset = aTextNode.Length();
   }
 
-  ErrorResult rv;
-  if ((uint32_t)aEndOffset != aTextNode.Length()) {
+  // Make the range an independent node.
+  nsCOMPtr<nsIContent> textNodeForTheRange = &aTextNode;
+
+  // Split at the end of the range.
+  EditorRawDOMPoint atEnd(textNodeForTheRange, aEndOffset);
+  if (!atEnd.IsEndOfContainer()) {
     // We need to split off back of text node
-    node = SplitNode(node, aEndOffset, rv);
-    NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
+    ErrorResult error;
+    textNodeForTheRange = SplitNode(atEnd, error);
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
   }
-  if (aStartOffset) {
+
+  // Split at the start of the range.
+  EditorRawDOMPoint atStart(textNodeForTheRange, aStartOffset);
+  if (!atStart.IsStartOfContainer()) {
     // We need to split off front of text node
-    SplitNode(node, aStartOffset, rv);
-    NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
+    ErrorResult error;
+    nsCOMPtr<nsIContent> newLeftNode = SplitNode(atStart, error);
+    if (NS_WARN_IF(error.Failed())) {
+      return error.StealNSResult();
+    }
+    Unused << newLeftNode;
   }
 
   // Look for siblings that are correct type of node
   nsAtom* nodeType = aDir == FontSize::incr ? nsGkAtoms::big
                                              : nsGkAtoms::small;
-  nsCOMPtr<nsIContent> sibling = GetPriorHTMLSibling(node);
+  nsCOMPtr<nsIContent> sibling = GetPriorHTMLSibling(textNodeForTheRange);
   if (sibling && sibling->IsHTMLElement(nodeType)) {
     // Previous sib is already right kind of inline node; slide this over
-    nsresult rv = MoveNode(node, sibling, -1);
+    nsresult rv = MoveNode(textNodeForTheRange, sibling, -1);
     NS_ENSURE_SUCCESS(rv, rv);
     return NS_OK;
   }
-  sibling = GetNextHTMLSibling(node);
+  sibling = GetNextHTMLSibling(textNodeForTheRange);
   if (sibling && sibling->IsHTMLElement(nodeType)) {
     // Following sib is already right kind of inline node; slide this over
-    nsresult rv = MoveNode(node, sibling, 0);
+    nsresult rv = MoveNode(textNodeForTheRange, sibling, 0);
     NS_ENSURE_SUCCESS(rv, rv);
     return NS_OK;
   }
 
   // Else reparent the node inside font node with appropriate relative size
-  nsCOMPtr<Element> newElement = InsertContainerAbove(node, nodeType);
+  nsCOMPtr<Element> newElement =
+    InsertContainerAbove(textNodeForTheRange, nodeType);
   NS_ENSURE_STATE(newElement);
 
   return NS_OK;

editor/libeditor/SelectionState.cpp

@@ -303,42 +303,47 @@ RangeUpdater::SelAdjDeleteNode(nsINode* aNode)
 }
 
 nsresult
-RangeUpdater::SelAdjSplitNode(nsIContent& aOldRightNode,
-                              int32_t aOffset,
+RangeUpdater::SelAdjSplitNode(nsIContent& aRightNode,
                               nsIContent* aNewLeftNode)
 {
   if (mLock) {
     // lock set by Will/DidReplaceParent, etc...
     return NS_OK;
   }
-  NS_ENSURE_TRUE(aNewLeftNode, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!aNewLeftNode)) {
+    return NS_ERROR_FAILURE;
+  }
+
   size_t count = mArray.Length();
   if (!count) {
     return NS_OK;
   }
 
-  nsCOMPtr<nsINode> parent = aOldRightNode.GetParentNode();
-  int32_t offset = parent ? parent->IndexOf(&aOldRightNode) : -1;
+  EditorRawDOMPoint atLeftNode(aNewLeftNode);
+  nsresult rv = SelAdjInsertNode(atLeftNode.Container(), atLeftNode.Offset());
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
-  // first part is same as inserting aNewLeftnode
-  nsresult rv = SelAdjInsertNode(parent, offset - 1);
-  NS_ENSURE_SUCCESS(rv, rv);
+  // If point in the ranges is in left node, change its container to the left
+  // node.  If point in the ranges is in right node, subtract numbers of
+  // children moved to left node from the offset.
+  int32_t lengthOfLeftNode = aNewLeftNode->Length();
+  for (RefPtr<RangeItem>& item : mArray) {
+    if (NS_WARN_IF(!item)) {
+      return NS_ERROR_FAILURE;
+    }
 
-  // next step is to check for range enpoints inside aOldRightNode
-  for (size_t i = 0; i < count; i++) {
-    RangeItem* item = mArray[i];
-    NS_ENSURE_TRUE(item, NS_ERROR_NULL_POINTER);
-
-    if (item->mStartContainer == &aOldRightNode) {
-      if (item->mStartOffset > aOffset) {
-        item->mStartOffset -= aOffset;
+    if (item->mStartContainer == &aRightNode) {
+      if (item->mStartOffset > lengthOfLeftNode) {
+        item->mStartOffset -= lengthOfLeftNode;
       } else {
         item->mStartContainer = aNewLeftNode;
       }
     }
-    if (item->mEndContainer == &aOldRightNode) {
-      if (item->mEndOffset > aOffset) {
-        item->mEndOffset -= aOffset;
+    if (item->mEndContainer == &aRightNode) {
+      if (item->mEndOffset > lengthOfLeftNode) {
+        item->mEndOffset -= lengthOfLeftNode;
       } else {
         item->mEndContainer = aNewLeftNode;
       }

editor/libeditor/SelectionState.h

@@ -112,8 +112,7 @@ public:
   nsresult SelAdjCreateNode(nsINode* aParent, int32_t aPosition);
   nsresult SelAdjInsertNode(nsINode* aParent, int32_t aPosition);
   void SelAdjDeleteNode(nsINode* aNode);
-  nsresult SelAdjSplitNode(nsIContent& aOldRightNode, int32_t aOffset,
-                           nsIContent* aNewLeftNode);
+  nsresult SelAdjSplitNode(nsIContent& aRightNode, nsIContent* aNewLeftNode);
   nsresult SelAdjJoinNodes(nsINode& aLeftNode,
                            nsINode& aRightNode,
                            nsINode& aParent,

editor/libeditor/SplitNodeTransaction.cpp

@@ -6,6 +6,7 @@
 #include "SplitNodeTransaction.h"
 
 #include "mozilla/EditorBase.h"         // for EditorBase
+#include "mozilla/EditorDOMPoint.h"     // for RangeBoundary, EditorRawDOMPoint
 #include "mozilla/dom/Selection.h"
 #include "nsAString.h"
 #include "nsDebug.h"                    // for NS_ASSERTION, etc.
@@ -16,13 +17,14 @@ namespace mozilla {
 
 using namespace dom;
 
-SplitNodeTransaction::SplitNodeTransaction(EditorBase& aEditorBase,
-                                           nsIContent& aNode,
-                                           int32_t aOffset)
+SplitNodeTransaction::SplitNodeTransaction(
+                        EditorBase& aEditorBase,
+                        const EditorRawDOMPoint& aStartOfRightNode)
   : mEditorBase(&aEditorBase)
-  , mExistingRightNode(&aNode)
-  , mOffset(aOffset)
+  , mStartOfRightNode(aStartOfRightNode)
 {
+  MOZ_DIAGNOSTIC_ASSERT(aStartOfRightNode.IsSet());
+  MOZ_DIAGNOSTIC_ASSERT(aStartOfRightNode.Container()->IsContent());
 }
 
 SplitNodeTransaction::~SplitNodeTransaction()
@@ -31,6 +33,7 @@ SplitNodeTransaction::~SplitNodeTransaction()
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(SplitNodeTransaction, EditTransactionBase,
                                    mEditorBase,
+                                   mStartOfRightNode,
                                    mParent,
                                    mNewLeftNode)
 
@@ -42,31 +45,59 @@ NS_INTERFACE_MAP_END_INHERITING(EditTransactionBase)
 NS_IMETHODIMP
 SplitNodeTransaction::DoTransaction()
 {
-  if (NS_WARN_IF(!mEditorBase)) {
+  if (NS_WARN_IF(!mEditorBase) ||
+      NS_WARN_IF(!mStartOfRightNode.IsSet())) {
     return NS_ERROR_NOT_INITIALIZED;
   }
+  MOZ_ASSERT(mStartOfRightNode.IsSetAndValid());
 
   // Create a new node
-  ErrorResult rv;
+  ErrorResult error;
   // Don't use .downcast directly because AsContent has an assertion we want
-  nsCOMPtr<nsINode> clone = mExistingRightNode->CloneNode(false, rv);
-  NS_ASSERTION(!rv.Failed() && clone, "Could not create clone");
-  NS_ENSURE_TRUE(!rv.Failed() && clone, rv.StealNSResult());
+  nsCOMPtr<nsINode> clone =
+    mStartOfRightNode.Container()->CloneNode(false, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  if (NS_WARN_IF(!clone)) {
+    return NS_ERROR_UNEXPECTED;
+  }
   mNewLeftNode = dont_AddRef(clone.forget().take()->AsContent());
-  mEditorBase->MarkNodeDirty(mExistingRightNode->AsDOMNode());
+  mEditorBase->MarkNodeDirty(mStartOfRightNode.Container()->AsDOMNode());
 
   // Get the parent node
-  mParent = mExistingRightNode->GetParentNode();
-  NS_ENSURE_TRUE(mParent, NS_ERROR_NULL_POINTER);
+  mParent = mStartOfRightNode.Container()->GetParentNode();
+  if (NS_WARN_IF(!mParent)) {
+    return NS_ERROR_FAILURE;
+  }
 
   // Insert the new node
-  rv = mEditorBase->SplitNodeImpl(*mExistingRightNode, mOffset, *mNewLeftNode);
+  mEditorBase->SplitNodeImpl(EditorDOMPoint(mStartOfRightNode),
+                             *mNewLeftNode, error);
+  // XXX Really odd.  The result of SplitNodeImpl() is respected only when
+  //     we shouldn't set selection.  Otherwise, it's overridden by the
+  //     result of Selection.Collapse().
   if (mEditorBase->GetShouldTxnSetSelection()) {
+    NS_WARNING_ASSERTION(!mEditorBase->Destroyed(),
+      "The editor has gone but SplitNodeTransaction keeps trying to modify "
+      "Selection");
     RefPtr<Selection> selection = mEditorBase->GetSelection();
-    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
-    rv = selection->Collapse(mNewLeftNode, mOffset);
+    if (NS_WARN_IF(!selection)) {
+      return NS_ERROR_FAILURE;
+    }
+    if (NS_WARN_IF(error.Failed())) {
+      // XXX This must be a bug.
+      error.SuppressException();
+    }
+    MOZ_ASSERT(mStartOfRightNode.Offset() == mNewLeftNode->Length());
+    EditorRawDOMPoint atEndOfLeftNode(mNewLeftNode, mNewLeftNode->Length());
+    selection->Collapse(atEndOfLeftNode, error);
   }
-  return rv.StealNSResult();
+
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -74,12 +105,16 @@ SplitNodeTransaction::UndoTransaction()
 {
   if (NS_WARN_IF(!mEditorBase) ||
       NS_WARN_IF(!mNewLeftNode) ||
-      NS_WARN_IF(!mParent)) {
+      NS_WARN_IF(!mParent) ||
+      NS_WARN_IF(!mStartOfRightNode.IsSet())) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   // This assumes Do inserted the new node in front of the prior existing node
-  return mEditorBase->JoinNodesImpl(mExistingRightNode, mNewLeftNode, mParent);
+  // XXX Perhaps, we should reset mStartOfRightNode with current first child
+  //     of the right node.
+  return mEditorBase->JoinNodesImpl(mStartOfRightNode.Container(), mNewLeftNode,
+                                    mParent);
 }
 
 /* Redo cannot simply resplit the right node, because subsequent transactions
@@ -90,37 +125,52 @@ NS_IMETHODIMP
 SplitNodeTransaction::RedoTransaction()
 {
   if (NS_WARN_IF(!mNewLeftNode) ||
-      NS_WARN_IF(!mParent)) {
+      NS_WARN_IF(!mParent) ||
+      NS_WARN_IF(!mStartOfRightNode.IsSet())) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  ErrorResult rv;
   // First, massage the existing node so it is in its post-split state
-  if (mExistingRightNode->GetAsText()) {
-    rv = mExistingRightNode->GetAsText()->DeleteData(0, mOffset);
-    NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
+  if (mStartOfRightNode.Container()->IsNodeOfType(nsINode::eTEXT)) {
+    Text* rightNodeAsText = mStartOfRightNode.Container()->GetAsText();
+    MOZ_DIAGNOSTIC_ASSERT(rightNodeAsText);
+    nsresult rv =
+      rightNodeAsText->DeleteData(0, mStartOfRightNode.Offset());
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   } else {
-    nsCOMPtr<nsIContent> child = mExistingRightNode->GetFirstChild();
+    nsCOMPtr<nsIContent> child = mStartOfRightNode.Container()->GetFirstChild();
     nsCOMPtr<nsIContent> nextSibling;
-    for (int32_t i=0; i < mOffset; i++) {
-      if (rv.Failed()) {
-        return rv.StealNSResult();
-      }
-      if (!child) {
+    for (uint32_t i = 0; i < mStartOfRightNode.Offset(); i++) {
+      // XXX This must be bad behavior.  Perhaps, we should work with
+      //     mStartOfRightNode::GetChildAtOffset().  Even if some children
+      //     before the right node have been inserted or removed, we should
+      //     move all children before the right node because user must focus
+      //     on the right node, so, it must be the expected behavior.
+      if (NS_WARN_IF(!child)) {
         return NS_ERROR_NULL_POINTER;
       }
       nextSibling = child->GetNextSibling();
-      mExistingRightNode->RemoveChild(*child, rv);
-      if (!rv.Failed()) {
-        mNewLeftNode->AppendChild(*child, rv);
+      ErrorResult error;
+      mStartOfRightNode.Container()->RemoveChild(*child, error);
+      if (NS_WARN_IF(error.Failed())) {
+        return error.StealNSResult();
+      }
+      mNewLeftNode->AppendChild(*child, error);
+      if (NS_WARN_IF(error.Failed())) {
+        return error.StealNSResult();
       }
       child = nextSibling;
     }
   }
   // Second, re-insert the left node into the tree
-  nsCOMPtr<nsIContent> refNode = mExistingRightNode;
-  mParent->InsertBefore(*mNewLeftNode, refNode, rv);
-  return rv.StealNSResult();
+  ErrorResult error;
+  mParent->InsertBefore(*mNewLeftNode, mStartOfRightNode.Container(), error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  return NS_OK;
 }
 
 

editor/libeditor/SplitNodeTransaction.h

@@ -6,6 +6,7 @@
 #ifndef SplitNodeTransaction_h
 #define SplitNodeTransaction_h
 
+#include "mozilla/EditorDOMPoint.h"      // for RangeBoundary, EditorRawDOMPoint
 #include "mozilla/EditTransactionBase.h" // for EditTxn, etc.
 #include "nsCOMPtr.h"                   // for nsCOMPtr
 #include "nsCycleCollectionParticipant.h"
@@ -27,14 +28,14 @@ class SplitNodeTransaction final : public EditTransactionBase
 {
 public:
   /**
-   * @param aEditorBase The provider of core editing operations
-   * @param aNode       The node to split
-   * @param aOffset     The location within aNode to do the split.  aOffset may
-   *                    refer to children of aNode, or content of aNode.  The
-   *                    left node will have child|content 0..aOffset-1.
+   * @param aEditorBase         The provider of core editing operations.
+   * @param aStartOfRightNode   The point to split.  Its container will be
+   *                            the right node, i.e., become the new node's
+   *                            next sibling.  And the point will be start
+   *                            of the right node.
    */
-  SplitNodeTransaction(EditorBase& aEditorBase, nsIContent& aNode,
-                       int32_t aOffset);
+  SplitNodeTransaction(EditorBase& aEditorBase,
+                       const EditorRawDOMPoint& aStartOfRightNode);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(SplitNodeTransaction,
@@ -51,13 +52,9 @@ protected:
 
   RefPtr<EditorBase> mEditorBase;
 
-  // The node to operate upon.
-  nsCOMPtr<nsIContent> mExistingRightNode;
-
-  // The offset into mExistingRightNode where its children are split.  mOffset
-  // is the index of the first child in the right node.  -1 means the new node
-  // gets no children.
-  int32_t mOffset;
+  // The container is existing right node (will be split).
+  // The point referring this is start of the right node after it's split.
+  RangeBoundary mStartOfRightNode;
 
   // The node we create when splitting mExistingRightNode.
   nsCOMPtr<nsIContent> mNewLeftNode;

editor/libeditor/TextEditor.cpp

@@ -444,35 +444,38 @@ TextEditor::CreateBRImpl(nsCOMPtr<nsIDOMNode>* aInOutParent,
   int32_t theOffset = *aInOutOffset;
   RefPtr<Element> brNode;
   if (IsTextNode(node)) {
-    EditorRawDOMPoint atNode(node);
-    if (NS_WARN_IF(!atNode.IsSetAndValid())) {
+    EditorRawDOMPoint pointToInsertBrNode(node);
+    if (NS_WARN_IF(!pointToInsertBrNode.IsSetAndValid())) {
       return NS_ERROR_FAILURE;
     }
     if (!theOffset) {
       // we are already set to go
     } else if (theOffset == static_cast<int32_t>(node->Length())) {
       // update offset to point AFTER the text node
-      atNode.AdvanceOffset();
+      pointToInsertBrNode.AdvanceOffset();
     } else {
+      MOZ_DIAGNOSTIC_ASSERT(theOffset < static_cast<int32_t>(node->Length()));
       // split the text node
-      ErrorResult rv;
-      SplitNode(*node->AsContent(), theOffset, rv);
-      if (NS_WARN_IF(rv.Failed())) {
-        return rv.StealNSResult();
+      EditorRawDOMPoint atStartOfNewLine(node, theOffset);
+      ErrorResult error;
+      nsCOMPtr<nsIContent> newLeftNode = SplitNode(atStartOfNewLine, error);
+      if (NS_WARN_IF(error.Failed())) {
+        return error.StealNSResult();
       }
-      atNode.Clear();
-      atNode.Set(node);
+      // The right node offset in the parent is now changed.  Recompute it.
+      pointToInsertBrNode.Set(node);
+      Unused << newLeftNode;
     }
     // create br
-    brNode = CreateNode(nsGkAtoms::br, atNode);
+    brNode = CreateNode(nsGkAtoms::br, pointToInsertBrNode);
     if (NS_WARN_IF(!brNode)) {
       return NS_ERROR_FAILURE;
     }
-    *aInOutParent = GetAsDOMNode(atNode.Container());
-    *aInOutOffset = atNode.Offset() + 1;
+    *aInOutParent = GetAsDOMNode(pointToInsertBrNode.Container());
+    *aInOutOffset = pointToInsertBrNode.Offset() + 1;
   } else {
-    EditorRawDOMPoint atTheOffset(node, theOffset);
-    brNode = CreateNode(nsGkAtoms::br, atTheOffset);
+    EditorRawDOMPoint pointToInsertBrNode(node, theOffset);
+    brNode = CreateNode(nsGkAtoms::br, pointToInsertBrNode);
     if (NS_WARN_IF(!brNode)) {
       return NS_ERROR_FAILURE;
     }

editor/nsIEditActionListener.idl

@@ -98,14 +98,13 @@ interface nsIEditActionListener : nsISupports{
 
   /**
    * Called after the editor splits a node.
-   * @param aExistingRightNode   the node to split.  It will become the new node's next sibling.
-   * @param aOffset              the offset of aExistingRightNode's content|children to do the split at
-   * @param aNewLeftNode         [OUT] the new node resulting from the split, becomes aExistingRightNode's previous sibling.
+   * @param aExistingRightNode   The node which was split.  It will become the
+   *                             next sibling of the new left node.
+   * @param aNewLeftNode         The new node resulting from the split, becomes
+   *                             the previous sibling of aExistingRightNode.
    */
   void DidSplitNode(in nsIDOMNode aExistingRightNode,
-                          in long      aOffset,
-                          in nsIDOMNode aNewLeftNode,
-                          in nsresult    aResult);
+                    in nsIDOMNode aNewLeftNode);
 
   /**
    * Called before the editor joins 2 nodes.

editor/txtsvc/nsTextServicesDocument.cpp

@@ -1620,15 +1620,9 @@ nsTextServicesDocument::DidDeleteNode(nsIDOMNode *aChild, nsresult aResult)
 }
 
 NS_IMETHODIMP
-nsTextServicesDocument::DidSplitNode(nsIDOMNode *aExistingRightNode,
-                                     int32_t     aOffset,
-                                     nsIDOMNode *aNewLeftNode,
-                                     nsresult    aResult)
+nsTextServicesDocument::DidSplitNode(nsIDOMNode* aExistingRightNode,
+                                     nsIDOMNode* aNewLeftNode)
 {
-  //**** KDEBUG ****
-  // printf("** SplitNode: 0x%.8x  %d  0x%.8x\n", aExistingRightNode, aOffset, aNewLeftNode);
-  // fflush(stdout);
-  //**** KDEBUG ****
   return NS_OK;
 }
 

editor/txtsvc/nsTextServicesDocument.h

@@ -105,10 +105,8 @@ public:
 
   NS_IMETHOD WillSplitNode(nsIDOMNode * aExistingRightNode,
                            int32_t      aOffset) override;
-  NS_IMETHOD DidSplitNode(nsIDOMNode *aExistingRightNode,
-                          int32_t     aOffset,
-                          nsIDOMNode *aNewLeftNode,
-                          nsresult    aResult) override;
+  NS_IMETHOD DidSplitNode(nsIDOMNode* aExistingRightNode,
+                          nsIDOMNode* aNewLeftNode) override;
 
   NS_IMETHOD WillJoinNodes(nsIDOMNode  *aLeftNode,
                            nsIDOMNode  *aRightNode,

extensions/spellcheck/src/mozInlineSpellChecker.cpp

@@ -1092,15 +1092,16 @@ NS_IMETHODIMP mozInlineSpellChecker::DidDeleteNode(nsIDOMNode *aChild, nsresult
   return NS_OK;
 }
 
-NS_IMETHODIMP mozInlineSpellChecker::WillSplitNode(nsIDOMNode *aExistingRightNode, int32_t aOffset)
+NS_IMETHODIMP
+mozInlineSpellChecker::WillSplitNode(nsIDOMNode* aExistingRightNode,
+                                     int32_t aOffset)
 {
   return NS_OK;
 }
 
 NS_IMETHODIMP
-mozInlineSpellChecker::DidSplitNode(nsIDOMNode *aExistingRightNode,
-                                    int32_t aOffset,
-                                    nsIDOMNode *aNewLeftNode, nsresult aResult)
+mozInlineSpellChecker::DidSplitNode(nsIDOMNode* aExistingRightNode,
+                                    nsIDOMNode* aNewLeftNode)
 {
   return SpellCheckBetweenNodes(aNewLeftNode, 0, aNewLeftNode, 0);
 }

gfx/layers/ipc/CompositorVsyncScheduler.cpp

@@ -117,6 +117,7 @@ CompositorVsyncScheduler::Destroy()
 
   CancelCurrentSetNeedsCompositeTask();
   CancelCurrentCompositeTask();
+  CancelCurrentVRTask();
 }
 
 void
@@ -133,6 +134,7 @@ CompositorVsyncScheduler::PostCompositeTask(TimeStamp aCompositeTimestamp)
     mCurrentCompositeTask = task;
     ScheduleTask(task.forget(), 0);
   }
+  MonitorAutoLock lockVR(mCurrentVRListenerTaskMonitor);
   if (mCurrentVRListenerTask == nullptr && VRListenerThreadHolder::Loop()) {
     RefPtr<CancelableRunnable> task = NewCancelableRunnableMethod<TimeStamp>(
       "layers::CompositorVsyncScheduler::DispatchVREvents",
@@ -242,6 +244,18 @@ CompositorVsyncScheduler::CancelCurrentCompositeTask()
   }
 }
 
+void
+CompositorVsyncScheduler::CancelCurrentVRTask()
+{
+  // This function is only called by CompositorVsyncScheduler::Destroy().
+  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
+  MonitorAutoLock lockVR(mCurrentVRListenerTaskMonitor);
+  if (mCurrentVRListenerTask) {
+    mCurrentVRListenerTask->Cancel();
+    mCurrentVRListenerTask = nullptr;
+  }
+}
+
 void
 CompositorVsyncScheduler::Composite(TimeStamp aVsyncTimestamp)
 {

gfx/layers/ipc/CompositorVsyncScheduler.h

@@ -83,6 +83,7 @@ private:
   void DispatchTouchEvents(TimeStamp aVsyncTimestamp);
   void DispatchVREvents(TimeStamp aVsyncTimestamp);
   void CancelCurrentSetNeedsCompositeTask();
+  void CancelCurrentVRTask();
 
   class Observer final : public VsyncObserver
   {

gfx/layers/ipc/SharedSurfacesChild.cpp

@@ -92,7 +92,6 @@ public:
 
   wr::ImageKey UpdateKey(WebRenderLayerManager* aManager,
                          wr::IpcResourceUpdateQueue& aResources,
-                         bool aForceUpdate,
                          uint32_t aGenerationId)
   {
     MOZ_ASSERT(aManager);
@@ -114,7 +113,7 @@ public:
         mKeys.RemoveElementAt(i);
       } else if (entry.mManager == aManager) {
         found = true;
-        if (aForceUpdate || entry.mGenerationId != aGenerationId) {
+        if (entry.mGenerationId != aGenerationId) {
           aManager->AddImageKeyForDiscard(entry.mImageKey);
           entry.mGenerationId = aGenerationId;
           entry.mImageKey = aManager->WrBridge()->GetNextImageKey();
@@ -151,7 +150,6 @@ SharedSurfacesChild::DestroySharedUserData(void* aClosure)
 SharedSurfacesChild::Share(SourceSurfaceSharedData* aSurface,
                            WebRenderLayerManager* aManager,
                            wr::IpcResourceUpdateQueue& aResources,
-                           bool aForceUpdate,
                            uint32_t aGenerationId,
                            wr::ImageKey& aKey)
 {
@@ -176,7 +174,7 @@ SharedSurfacesChild::Share(SourceSurfaceSharedData* aSurface,
     data->SetId(manager->GetNextExternalImageId());
   } else if (data->IsShared()) {
     // It has already been shared with the GPU process, reuse the id.
-    aKey = data->UpdateKey(aManager, aResources, aForceUpdate, aGenerationId);
+    aKey = data->UpdateKey(aManager, aResources, aGenerationId);
     return NS_OK;
   }
 
@@ -194,7 +192,7 @@ SharedSurfacesChild::Share(SourceSurfaceSharedData* aSurface,
   if (pid == base::GetCurrentProcId()) {
     SharedSurfacesParent::AddSameProcess(data->Id(), aSurface);
     data->MarkShared();
-    aKey = data->UpdateKey(aManager, aResources, aForceUpdate, aGenerationId);
+    aKey = data->UpdateKey(aManager, aResources, aGenerationId);
     return NS_OK;
   }
 
@@ -229,7 +227,7 @@ SharedSurfacesChild::Share(SourceSurfaceSharedData* aSurface,
                                 SurfaceDescriptorShared(aSurface->GetSize(),
                                                         aSurface->Stride(),
                                                         format, handle));
-  aKey = data->UpdateKey(aManager, aResources, aForceUpdate, aGenerationId);
+  aKey = data->UpdateKey(aManager, aResources, aGenerationId);
   return NS_OK;
 }
 
@@ -237,7 +235,6 @@ SharedSurfacesChild::Share(SourceSurfaceSharedData* aSurface,
 SharedSurfacesChild::Share(ImageContainer* aContainer,
                            WebRenderLayerManager* aManager,
                            wr::IpcResourceUpdateQueue& aResources,
-                           bool aForceUpdate,
                            wr::ImageKey& aKey)
 {
   MOZ_ASSERT(NS_IsMainThread());
@@ -266,7 +263,7 @@ SharedSurfacesChild::Share(ImageContainer* aContainer,
 
   auto sharedSurface = static_cast<SourceSurfaceSharedData*>(surface.get());
   return Share(sharedSurface, aManager, aResources,
-               aForceUpdate, generation, aKey);
+               generation, aKey);
 }
 
 /* static */ void