Refactor bookmark-service - getNativeContainerIds into a shared webext- impl.

Created two new abstract methods:
getNativeContainerInfo() -> returns browser-specific info (id + throwIfNotFound) for a given BookmarkContainer
getDefaultNativeContainerCandidates() -> returns the candidates for being the default containers for creation of unsupported containers

Field unsupportedContainers was refactored into getUnsupportedContainers() method. Added getSupportedContainers().
NativeContainersInfo.platformDefaultBookmarksNodeId renamed to .defaultNativeContainerId

Refactored and moved ensureContainersExist(...) to webext- impl. It uses the getSupportedContainers()

Created a new webext- impl. method identifySupportedContainers(). This method initializes the supported containers cache.
This method must be called at least once after the extension is loaded, before a call to getSupportedContainers(), getUnsupportedContainers() or ensureContainersExist (...) is attempted.

Currently, identifySupportedContainers() is called before any process*Sync() in bookmark-sync-provider can take place, but should be probably moved to some module/extension init code. Maybe the whole "supported-containers" logic should be moved to different (new?) service.

Author: michkot

src/modules/android/android-shared/android-bookmark/android-bookmark.service.ts

@@ -30,6 +30,10 @@ export default class AndroidBookmarkService implements BookmarkService {
     return bookmarks;
   }
 
+  identifySupportedContainers(): ng.IPromise<void> {
+    return this.methodNotApplicable();
+  }
+
   methodNotApplicable(): ng.IPromise<any> {
     // Unused for this platform
     return this.$q.resolve();

src/modules/shared/bookmark/bookmark.enum.ts

@@ -13,19 +13,11 @@ enum BookmarkContainer {
   Toolbar = '[xbs] Toolbar'
 }
 
-// TODO: maybe remove this new code and use solely (un)supported bookmark detection at runtime
-//  OR! always create all containers, so that merging in the future is simpler?
-const MandatoryBookmarkContainers: Array<BookmarkContainer> = [
-  BookmarkContainer.Other,
-  BookmarkContainer.Mobile,
-  BookmarkContainer.Toolbar
-];
-
 enum BookmarkType {
   Bookmark = 'bookmark',
   Container = 'container',
   Folder = 'folder',
   Separator = 'separator'
 }
 
-export { BookmarkChangeType, BookmarkContainer, BookmarkType, MandatoryBookmarkContainers };
+export { BookmarkChangeType, BookmarkContainer, BookmarkType };

src/modules/shared/bookmark/bookmark.interface.ts

@@ -41,6 +41,7 @@ export interface BookmarkService {
   clearNativeBookmarks: () => ng.IPromise<void>;
   createNativeBookmarksFromBookmarks: (bookmarks: Bookmark[]) => ng.IPromise<number>;
   ensureContainersExist: (bookmarks: Bookmark[]) => Bookmark[];
+  identifySupportedContainers(): ng.IPromise<void>;
   processNativeChangeOnBookmarks: (changeInfo: BookmarkChange, bookmarks: Bookmark[]) => ng.IPromise<Bookmark[]>;
   processChangeOnNativeBookmarks: (
     id: number,

src/modules/shared/sync/bookmark-sync-provider/bookmark-sync-provider.service.ts

@@ -137,24 +137,26 @@ export default class BookmarkSyncProviderService implements SyncProvider {
   }
 
   processSync(sync: Sync): ng.IPromise<ProcessSyncResult> {
-    // Process sync
-    switch (sync.type) {
-      // Sync native bookmarks to service
-      case SyncType.Remote:
-        return this.processRemoteSync(sync);
-      // Overwrite native bookmarks with synced bookmarks
-      case SyncType.Local:
-        return this.processLocalSync(sync);
-      // Sync bookmarks to service and overwrite native bookmarks
-      case SyncType.LocalAndRemote:
-        return this.processLocalAndRemoteSync(sync);
-      // Upgrade sync to current version
-      case SyncType.Upgrade:
-        return this.processUpgradeSync();
-      // Ambiguous sync
-      default:
-        throw new Exceptions.AmbiguousSyncRequestException();
-    }
+    return this.bookmarkSvc.identifySupportedContainers().then(() => {
+      // Process sync
+      switch (sync.type) {
+        // Sync native bookmarks to service
+        case SyncType.Remote:
+          return this.processRemoteSync(sync);
+        // Overwrite native bookmarks with synced bookmarks
+        case SyncType.Local:
+          return this.processLocalSync(sync);
+        // Sync bookmarks to service and overwrite native bookmarks
+        case SyncType.LocalAndRemote:
+          return this.processLocalAndRemoteSync(sync);
+        // Upgrade sync to current version
+        case SyncType.Upgrade:
+          return this.processUpgradeSync();
+        // Ambiguous sync
+        default:
+          throw new Exceptions.AmbiguousSyncRequestException();
+      }
+    });
   }
 
   processLocalAndRemoteSync(sync: Sync): ng.IPromise<ProcessSyncResult> {

src/modules/webext/chromium/shared/chromium-bookmark/chromium-bookmark.service.ts

@@ -2,34 +2,21 @@ import angular from 'angular';
 import { Injectable } from 'angular-ts-decorators';
 import autobind from 'autobind-decorator';
 import { Bookmarks as NativeBookmarks, browser } from 'webextension-polyfill-ts';
-import {
-  BookmarkChangeType,
-  BookmarkContainer,
-  BookmarkType,
-  MandatoryBookmarkContainers
-} from '../../../../shared/bookmark/bookmark.enum';
+import { BookmarkChangeType, BookmarkContainer, BookmarkType } from '../../../../shared/bookmark/bookmark.enum';
 import {
   AddNativeBookmarkChangeData,
-  Bookmark,
   BookmarkChange,
   ModifyNativeBookmarkChangeData,
   MoveNativeBookmarkChangeData
 } from '../../../../shared/bookmark/bookmark.interface';
 import * as Exceptions from '../../../../shared/exception/exception';
 import Globals from '../../../../shared/global-shared.constants';
 import { WebpageMetadata } from '../../../../shared/global-shared.interface';
-import { NativeContainersInfo } from '../../../shared/webext-bookmark/NativeContainersInfo';
 import WebExtBookmarkService from '../../../shared/webext-bookmark/webext-bookmark.service';
 
 @autobind
 @Injectable('BookmarkService')
 export default class ChromiumBookmarkService extends WebExtBookmarkService {
-  otherBookmarksNodeTitle = 'Other bookmarks';
-  toolbarBookmarksNodeTitle = 'Bookmarks bar';
-  menuBookmarksNodeTitle = 'Menu bookmarks';
-  mobileBookmarksNodeTitle = 'Mobile bookmarks';
-  unsupportedContainers = [BookmarkContainer.Menu, BookmarkContainer.Mobile];
-
   convertNativeBookmarkToSeparator(
     bookmark: NativeBookmarks.BookmarkTreeNode
   ): ng.IPromise<NativeBookmarks.BookmarkTreeNode> {
@@ -134,32 +121,6 @@ export default class ChromiumBookmarkService extends WebExtBookmarkService {
       });
   }
 
-  // TODO: unify with firefox somehow?
-  // probably auto-detect all supported containers in getNativeContainer (maybe call it explicitely for detection?)
-  // and then ensure that all supported containers are created
-  ensureContainersExist(bookmarks: Bookmark[]): Bookmark[] {
-    if (angular.isUndefined(bookmarks)) {
-      return;
-    }
-
-    // Add supported containers
-    const bookmarksToReturn = angular.copy(bookmarks);
-    MandatoryBookmarkContainers.forEach((element) => {
-      this.bookmarkHelperSvc.getContainer(element, bookmarksToReturn, true);
-    });
-
-    // Return sorted containers
-    return bookmarksToReturn.sort((x, y) => {
-      if (x.title < y.title) {
-        return -1;
-      }
-      if (x.title > y.title) {
-        return 1;
-      }
-      return 0;
-    });
-  }
-
   getNativeBookmarksWithSeparators(
     nativeBookmarks: NativeBookmarks.BookmarkTreeNode[]
   ): NativeBookmarks.BookmarkTreeNode[] {
@@ -172,90 +133,83 @@ export default class ChromiumBookmarkService extends WebExtBookmarkService {
     return nativeBookmarks;
   }
 
-  // TODO: unify/share more code with firefox
-  getNativeContainerIds(): ng.IPromise<NativeContainersInfo> {    
-    const containerIds = new NativeContainersInfo();
-    // Populate container ids
-    return browser.bookmarks.getTree().then((tree) => {
-      // Get the root child nodes
-      let otherBookmarksNode = tree[0].children.find((x) => {
-        return x.title === this.otherBookmarksNodeTitle;
-      });
-      let toolbarBookmarksNode = tree[0].children.find((x) => {
-        return x.title === this.toolbarBookmarksNodeTitle;
-      });
-      let menuBookmarksNode = tree[0].children.find((x) => {
-        return x.title === this.menuBookmarksNodeTitle;
-      });
-      let mobileBookmarksNode = tree[0].children.find((x) => {
-        return x.title === this.mobileBookmarksNodeTitle;
-      });
+  browserDetection: { isOpera: boolean };
+  getBrowserDetection() {
+    if (this.browserDetection) return this.browserDetection;
 
-      // TODO: improve this logic
-      const defaultBookmarksNode = otherBookmarksNode || mobileBookmarksNode;
-      if (!defaultBookmarksNode) {
-        // coulnd not find a default container to create folders to place other containers in
-        throw new Exceptions.ContainerNotFoundException();
-      }
+    const browserDetection: any = {};
+    browserDetection.isChromeLike = this.utilitySvc.isChromeLikeBrowser();
+    browserDetection.isOpera = this.utilitySvc.isOperaBrowser();
+    browserDetection.isEdgeChromium = this.utilitySvc.isEdgeChromiumBrowser();
 
-      // TODO: FINISH THIS!
-      // is related to createNativeBookmarksFromBookmarks
-      // HACK!!!!
-      this.unsupportedContainers = [];
+    this.browserDetection = browserDetection;
+    return this.browserDetection;
+  }
 
-      // Check for unsupported containers
-      if (!otherBookmarksNode) {
-        this.logSvc.logWarning('Unsupported container: other bookmarks');
-        // HACK!!!!
-        this.unsupportedContainers.push(BookmarkContainer.Other);
-        otherBookmarksNode = defaultBookmarksNode.children.find((x) => {
-          return x.title === BookmarkContainer.Other;
-        });
-      }
-      if (!toolbarBookmarksNode) {
-        this.logSvc.logWarning('Unsupported container: toolbar bookmarks');
-        // HACK!!!!
-        this.unsupportedContainers.push(BookmarkContainer.Toolbar);
-        toolbarBookmarksNode = defaultBookmarksNode.children.find((x) => {
-          return x.title === BookmarkContainer.Toolbar;
-        });
-      }
-      if (!menuBookmarksNode) {
-        this.logSvc.logWarning('Unsupported container: menu bookmarks');
-        // HACK!!!!
-        this.unsupportedContainers.push(BookmarkContainer.Menu);
-        menuBookmarksNode = defaultBookmarksNode.children.find((x) => {
-          return x.title === BookmarkContainer.Menu;
-        });
-      }
-      if (!mobileBookmarksNode) {
-        this.logSvc.logWarning('Unsupported container: mobile bookmarks');
-        // HACK!!!!
-        this.unsupportedContainers.push(BookmarkContainer.Mobile);
-        mobileBookmarksNode = defaultBookmarksNode.children.find((x) => {
-          return x.title === BookmarkContainer.Mobile;
+  chromiumSupportedContainersInfo = {
+    map: new Map<BookmarkContainer, { id: string; throwIfNotFound: boolean }>([
+      [BookmarkContainer.Toolbar, { id: '1', throwIfNotFound: false }],
+      [BookmarkContainer.Other, { id: '2', throwIfNotFound: false }],
+      [BookmarkContainer.Mobile, { id: '3', throwIfNotFound: false }]
+    ]),
+    default: [BookmarkContainer.Other, BookmarkContainer.Mobile]
+  };
+
+  operaSupportedContainersInfo = {
+    map: new Map<BookmarkContainer, { id: string; throwIfNotFound: boolean }>([
+      [BookmarkContainer.Toolbar, { id: 'bookmarks_bar', throwIfNotFound: false }],
+      [BookmarkContainer.Other, { id: 'other', throwIfNotFound: true }]
+      // [, { id: 'unsorted', throwIfNotFound: false }],
+      // [, { id: 'user_root', throwIfNotFound: false }],
+      // [, { id: 'shared', throwIfNotFound: false }],
+      // [, { id: 'trash', throwIfNotFound: false }],
+      // [, { id: 'speed_dial', throwIfNotFound: false }],
+    ]),
+    default: [BookmarkContainer.Other]
+  };
+
+  getNativeContainerInfo(containerName: BookmarkContainer): ng.IPromise<{ id?: string; throwIfNotFound: boolean }> {
+    const browserDetection = this.getBrowserDetection();
+    if (browserDetection.isOpera) {
+      const getByName: (
+        id: string,
+        callback: (node: NativeBookmarks.BookmarkTreeNode) => void
+      ) => void = (browser as any).bookmarks.getRootByName;
+
+      const baseInfo = this.operaSupportedContainersInfo.map.get(containerName);
+      if (baseInfo) {
+        return this.$q((resolve) => {
+          getByName(baseInfo.id, (node) => {
+            resolve({ id: node.id, throwIfNotFound: baseInfo.throwIfNotFound });
+          });
         });
+        // eslint-disable-next-line no-else-return
+      } else {
+        return this.$q.resolve({ id: undefined, throwIfNotFound: false });
       }
+      // eslint-disable-next-line no-else-return
+    } else {
+      return browser.bookmarks.getTree().then((tree) => {
+        const baseInfo = this.chromiumSupportedContainersInfo.map.get(containerName);
+        let info: { id?: string; throwIfNotFound: boolean };
+        if (baseInfo && tree[0].children!.find((x) => x.id === baseInfo.id)) {
+          info = { ...baseInfo }; // make a copy
+        } else {
+          info = { id: undefined, throwIfNotFound: false };
+        }
+        return info;
+      });
+    }
+  }
 
-      // Add container ids to result
-      {
-        // must be always defined!
-        containerIds.platformDefaultBookmarksNodeId = defaultBookmarksNode.id;
-      }
-      if (!angular.isUndefined(otherBookmarksNode)) {
-        containerIds.set(BookmarkContainer.Other, otherBookmarksNode.id);
-      }
-      if (!angular.isUndefined(toolbarBookmarksNode)) {
-        containerIds.set(BookmarkContainer.Toolbar, toolbarBookmarksNode.id);
-      }
-      if (!angular.isUndefined(menuBookmarksNode)) {
-        containerIds.set(BookmarkContainer.Menu, menuBookmarksNode.id);
-      }
-      if (!angular.isUndefined(mobileBookmarksNode)) {
-        containerIds.set(BookmarkContainer.Mobile, mobileBookmarksNode.id);
-      }
-      return containerIds;
-    });
+  getDefaultNativeContainerCandidates(): BookmarkContainer[] {
+    const browserDetection = this.getBrowserDetection();
+    if (browserDetection.isOpera) {
+      return this.operaSupportedContainersInfo.default;
+      // eslint-disable-next-line no-else-return
+    } else {
+      return this.chromiumSupportedContainersInfo.default;
+    }
   }
 
   isSeparator(nativeBookmark: NativeBookmarks.BookmarkTreeNode): boolean {